Open smithfred opened 6 years ago
smithfred
commented
6 years ago Having at least tags would be very useful personally as I'm RPM-packaging this for Fedora (privately at the moment, but I don't mind potentially making them public in future).
RPM spec files need a source archive URL, and GitHub generates semantically named tarballs for tags, instead of commit-named zips, e.g.:
https://github.com/kurogetsusai/firefox-gnome-theme/archive/v58.0.tar.gz
vs.
lunakurame
commented
6 years ago Actually I've been thinking a bit about this, we already have support for Firefox 57, Mozilla isn't gonna change that version anymore, so it's mostly stable, but we might want to still fix some minor bugs and add new features like the code you wrote for private windows, URL bar autocomplete etc, so tagging the "last supported" version of this theme for a specific Firefox version wouldn't be much helpful from my point of view.
I'd like to avoid using separate branches for each Firefox version, I worked on a few projects which did that and the merge commits were a real pain, it's hard to keep track of merging a specific feature to specific branches and not end up with more merge commits than normal commits. So no branches.
Another problem I noticed is the theme.css file is getting long and messy, so my idea is to split it into several files, eg. toolbars.css, headerbars.css and put version specific hacks into separate files too, like ff57-downloads-popup.css, then we can replace the theme.css file with a file containing just a bunch of @imports, which also allows us to have a separate file for each Firefox version, so we can still share most of the code between versions and apply version specific fixes without repeating / cluttering the code. That means we will support all Firefox versions starting from 57 without much pain. A user will have to pick a Firefox version and a GNOME theme version they want to use in the userChome.css file (or somewhere else if we move the configuration like you said in #18). I don't like adding those switches in userChrome.css, but that's a small price for solving the versioning issue and it's finally gonna be clear on which FF version is the code supposed to run. I already started splitting the theme.css file, but I'd like to hear your opinion too.
If you want nice tarballs from GitHub, I can release a 1.0.0 version after I resolve this and #18, and #21 and after that we will just follow the semver specs and release a version from time to time. Does it help you with these RPM packages? I have no idea how they work, so I'm not sure.
And finally about our standard setups. I'll define something later, but the general idea is we support all released (so no beta / nightly) stock Firefox versions, if some distros have their own patches like Fedora (which will disappear soon, after FF 59 gets released), we might support them too, but I can't debug Firefox in a VirtualBox, so I probably won't be able to do much. I'll count on other contributors, Rafael wrote most of our CSD support for Firefox 57 since I wasn't able to run Fedora. I'm also not sure if my soon-to-be-commited FF58 fixes are gonna look good on Fedora, so I'll need someone's feedback again. Also I can't write themes for every new GNOME version. I'll do what I can, but it would be nice if other people contributed too and wrote themes for their GNOME version if they want it. And we support only normal theme density.
I'll write a CONTRIBUTING.md file when I've got some time, so hopefully that helps potential contributors understand the code structure and submit proper pull requests.
smithfred
commented
6 years ago we already have support for Firefox 57, Mozilla isn't gonna change that version anymore, so it's mostly stable
No AFAIK 57 is now obsolete/unsupported (since the moment 58 was released). No distro should be continuing to provide 57.x releases (unless they're effectively making a custom downstream ESR for some dumb reason ;)), they should either have upgraded to 58, or be using ESR (52). I'd suggest dropping support for obsolete and potentially vulnerable Firefox versions immediately, and focus on latest/ESR only, i.e. currently 58 and 52.
That would also allow for a pretty simple branched setup, at most ESR, current, next.
so my idea is to split it into several files
Presumably there's no ongoing performance penalty, just a probably minuscule bit extra startup time. Seems like a decent idea in general, as long as the divisions end up maintainable without grey areas.
If you want nice tarballs from GitHub
Just annotated tags is fine; GH's "release" thing isn't necessary (it's a proprietary GH feature too, not a standard Git thing).
Does it help you with these RPM packages?
It's doable as is (already packaged it via a commit snapshot zip); having a tarball URL that corresponds to a version rather than a commit ID just makes it a bit nicer.
Having a version (i.e. annotated tag for tarball purposes) that maps to the Firefox version also makes packaging simpler, since you just bump the version of the RPM spec. file (58, 59 etc.) and set it to depend on firefox-$version, source at github/blah/$version.tar.gz etc.
With a simplified version support/branching approach (ESR+current), I really think that following Firefox versioning is the way to go, since it's more semantic in this case than semver ;)
but I can't debug Firefox in a VirtualBox
What distro/GNOME/Firefox are you on? Ubuntu 16.04 LTS/GNOME 3.18/Firefox 58 at a guess?
What sort of debugging are you trying but unable to do? Have you tried in virt-manager or gnome-boxes (i.e. KVM)?
Also I can't write themes for every new GNOME version
That's why I suggested just latest GNOME plus whatever Ubuntu LTS is on (since Ubuntu is the old/mangled-GNOME-shipping 800 lb gorilla... :J)
lunakurame
commented
6 years ago Ok, so I decided. Old versions will get tags (I'll tag what we have now as 57 soon). Most changes will probably apply to both stable and beta and I don't wanna merge both branches after every single commit, so we're not using branches for versions. We might wanna use feature branches if they are necessary, but I don't think they will be. master is the current stable Firefox version, fixes for the next beta will go to separate files like our CSD support (csd.css vs fedora-csd.css), except they will be named more like csd-59.css, where 59 is current beta. They are experimental, when that version becomes stable and the old one is tagged, those files will be removed and their contents moved to appropriate files. We don't have a LTS yet, 59 will be the first ESR, so I think I'll just create a esr branch, but it won't receive new features, just major bugfixes.
What distro/GNOME/Firefox are you on? Ubuntu 16.04 LTS/GNOME 3.18/Firefox 58 at a guess?
Yes, except I'm still on Firefox 57.
What sort of debugging are you trying but unable to do? Have you tried in virt-manager or gnome-boxes (i.e. KVM)?
Just opening the inspector for the browser's UI (ctrl+shift+alt+i when remote debug is enabled), I only tried VirtualBox so far but I'll try something else when i have some time. Also when FF58 becomes obsolete, this won't be necessary anymore, since other distros just ship stock FF and Fedora's only reason to patch it will disappear too, since FF59 supports CSD, so it's not an important issue anyway.
Per #21, would be useful to have tags for specific versions, and a policy for what master tracks (nightly? beta? release?)
Tags would be e.g. 58.0 for the release of the theme for FF58, then 58.1 for the first bugfix release of the theme for FF58 etc. Fixes for "old" versions of Firefox (i.e. FF versions older than whatever master tracks) would go into version-specific branches.
So maybe master applies against nightly, then there's a 59 branch and a 58 branch.
As mentioned on #21, Fedora's CSD backports can be detected so don't need a separate branch.
Also, would be good to have some definition of a "standard" setup the theme is tested against - Adwaita Light/Dark desktop theme (not "global dark"); normal Firefox theme density; fresh Firefox profile; supported toolbar layouts; supported GNOME version(s); supported distributions. There are a few "fuck knows why this doesn't work for some people, twiddle with this stuff randomly to get it work" things in the theme at the moment.
For distros tested against I'd suggest at least Ubuntu and Fedora - Ubuntu since it's the behemoth, Fedora because it pretty much offers standard/latest GNOME vs. Ubuntu's mess of old versions, GNOME patches etc. etc. The Firefox CSD patches/backports are unusual, Fedora doesn't usually do early downstream stuff like that.
For supported GNOME versions I'd suggest latest release + maybe current Ubuntu LTS GNOME version (again, the behemoth factor).