Remove liblarch #1145
Remove liblarch #1145
Conversation
|
LGTM, I think you had merge rights? Feel free to undraft and merge this anytime. |
diegogangl
added this to the 0.7: "Getting things Gradius — Destroy the core!" milestone
diegogangl
added
documentation
There was a problem hiding this comment.
Just two nitpicks:
- The snap manifest still uses liblarch: https://github.com/SqAtx/gtg/blob/cae22e1cbaa2859e972e4e615cf0113337de8c70/snap/snapcraft.yaml#L51
- There is some incorrect type hint using liblarch in MainWindow: https://github.com/SqAtx/gtg/blob/cae22e1cbaa2859e972e4e615cf0113337de8c70/GTG/gtk/browser/main_window.py#L797
SqAtx
force-pushed
the
remove_liblarch
branch
from
cae22e1
to
8ec85dd
Compare
|
Thanks; I've updated these two places. liblarch is mentioned in a few other places. I thought we could remove these mentions when we come across them, as I'm not 100% sure how to deal with all of them. My goal here was mostly to remove it from the README and the list of dependencies. |
I do not :) I think this can be merged. I haven't re-tested it, but the only changes I made since my last tests are to comments, type hints, and the Snap packaging. |
| @@ -46,13 +46,3 @@ parts: | |||
| -e 's|Icon=.*|Icon=${SNAP}/usr/share/icons/hicolor/scalable/apps/org.gnome.GTG.svg|' | |||
| after: | |||
| - python-deps | |||
|
|
|||
| python-deps: | |||
| source: https://github.com/getting-things-gnome/liblarch.git | |||
There was a problem hiding this comment.
I have absolutely no experience with snap yet I think you removed here to much. I guess only line 51 (source: [...] ) it should be removed here.
There was a problem hiding this comment.
@sergiusens, would you be so kind to check the snapcraft file? You were the last one to contribute to it.
There was a problem hiding this comment.
Hm yeah it's possible. The other lines kind of look like the dependencies of liblarch, but they may be dependencies of GTG.
I'm not super fussed about this file because I don't think we release snaps regularly, and I don't think we've done it since the GTK4 refactor. So there is a good chance it doesn't work at all.
Happy to go with your suggestion, but if there is further debate I'd like to defer this work and just not touch the file.
I think it has been unused since the move to GTK4. Tested by creating and running the flatpak.
SqAtx
force-pushed
the
remove_liblarch
branch
from
8ec85dd
to
da4fcec
Compare
|
LGTM, merging. Thanks! |
I think it has been unused since the move to GTK4.
Tested by creating and running the flatpak.