[Bug 1377631] New: Review Request: gnome-shell-extension-netspeed - A gnome-shell extension to show speed of the internet
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
Bug ID: 1377631 Summary: Review Request: gnome-shell-extension-netspeed - A gnome-shell extension to show speed of the internet Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: mgansser@alice.de QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://www.dropbox.com/s/0cfb4boq3dhclu9/gnome-shell-extension-netspeed.spe... SRPM URL: https://www.dropbox.com/s/qsc4khyihlxnm9h/gnome-shell-extension-netspeed-3.2...
Description: Add an Internet speed indicator to status area. Fedora Account System Username: martinkg
%changelog * Fri Sep 16 2016 Martin Gansser martinkg@fedoraproject.org - 3.20-1.20160806git16a25ec - Update to 3.20
rpmlint -i gnome-shell-extension-netspeed.spec /home/martin/rpmbuild/SRPMS/gnome-shell-extension-netspeed-3.20-1.20160806git16a25ec.fc24.src.rpm /home/martin/rpmbuild/RPMS/noarch/gnome-shell-extension-netspeed-3.20-1.20160806git16a25ec.fc24.noarch.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings.
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
leigh scott leigh123linux@googlemail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |leigh123linux@googlemail.co | |m
--- Comment #1 from leigh scott leigh123linux@googlemail.com --- Release is wrong as it's git
change from
1.%{gitdate}git%{shortcommit}%{?dist}
to
0.1.%{gitdate}git%{shortcommit}%{?dist}
As for the version, don't guess and make it up.
Version: 3.20
3.16 was the last release so use
Version: 3.17
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #2 from MartinKG mgansser@alice.de --- Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/gnome-shell-extension-netspee... SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/gnome-shell-extension-netspee...
%changelog * Wed Sep 21 2016 Martin Gansser martinkg@fedoraproject.org - 3.17-0.2.20160806git16a25ec - Use correct git release version - Use correct version 3.17
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #3 from MartinKG mgansser@alice.de --- Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/gnome-shell-extension-netspee... SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/gnome-shell-extension-netspee...
%changelog * Wed Sep 21 2016 Martin Gansser martinkg@fedoraproject.org - 3.17-0.3.20160806git16a25ec - Add LICENSE file
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
Sam P. sam@sam.today changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |sam@sam.today
--- Comment #4 from Sam P. sam@sam.today --- What is the status of the upstream for this extension? The spec applies the patch version 26 patch [1], which is noted as "approved" and "active" on extensions.gnome.org. It was approved by the user EGO "hedayaty".
But the package is based on the git repository, which contains version 25 [2]. However the git repo is maintained by GitHub user "hedayaty" - the same person who applied the patch on EGO.
I opened an issue asking them what is going on, because right now it is confusing and odd: https://github.com/hedayaty/NetSpeed/issues/52
Anyway, the spec file looks good to my untrained eyes and the extension works with the built rpm.
[1] https://extensions.gnome.org/review/6097 [2] https://github.com/hedayaty/NetSpeed/blob/master/metadata.json#L9
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #5 from Andrew Toskin andrew@tosk.in --- I don't know where Leigh came up with incrementing the version from 3.16 to 3.17. As Sam said, according to the extension's metadata.json file, the version is either 25 or 26, depending on whether you're getting it from the extensions website, or from GitHub... And since the spec uses commit tags on GitHub as the Source0, I suppose it's a "pre-release" of version 26 (even though 26 has been released elsewhere).
Other issues:
[x]: glib-compile-schemas is run in %postun and %posttrans if package has *.gschema.xml files.
You correctly compile the gschema in the system glib directory, so you don't need to include the precompiled copy (or even the directory) at: %{builddir}/%{_datadir}/gnome-shell/extensions/%{uuid}/schemas/
You can remove it at the end of the %install section, probably just before %find_lang would be best.
[?]: The spec file handles locales properly.
Looks to me like the locale files are all in the right place, but when I changed my system language to French and German, the settings widget was still all in English.
This is *probably* a problem upstream, since the version installed from the EGO site doesn't seem to translate either.
https://github.com/hedayaty/NetSpeed/issues/55
[!]: Requires correct, justified where necessary.
Should require GNOME Shell 3.10+, according to metadata.json's list of compatible versions of GNOME.
For any GNOME Shell extension, you should also list gnome-shell-extension-common as a dependency.
[x]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
There's an open issue about this: https://github.com/hedayaty/NetSpeed/issues/50
The patch which adds newer versions of GNOME to the compatibility list should hopefully be taken care of upstream:
https://github.com/hedayaty/NetSpeed/issues/42
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #6 from leigh scott leigh123linux@googlemail.com --- (In reply to Andrew Toskin from comment #5)
I don't know where Leigh came up with incrementing the version from 3.16 to 3.17.
The changelog declares the current version as 3.16
https://github.com/hedayaty/NetSpeed/blob/master/CHANGELOG
so packaging any git snapshot should be bumped by +1 to the minor
As Sam said, according to the extension's metadata.json file, the version is either 25 or 26, depending on whether you're getting it from the extensions website, or from GitHub... And since the spec uses commit tags on GitHub as the Source0, I suppose it's a "pre-release" of version 26 (even though 26 has been released elsewhere).
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
mgansser@alice.de mgansser@online.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mgansser@online.de
--- Comment #7 from mgansser@alice.de mgansser@online.de --- (In reply to Andrew Toskin from comment #5)
I don't know where Leigh came up with incrementing the version from 3.16 to 3.17. As Sam said, according to the extension's metadata.json file, the version is either 25 or 26, depending on whether you're getting it from the extensions website, or from GitHub... And since the spec uses commit tags on GitHub as the Source0, I suppose it's a "pre-release" of version 26 (even though 26 has been released elsewhere).
Other issues:
[x]: glib-compile-schemas is run in %postun and %posttrans if package has *.gschema.xml files.
You correctly compile the gschema in the system glib directory, so you don't need to include the precompiled copy (or even the directory) at: %{builddir}/%{_datadir}/gnome-shell/extensions/%{uuid}/schemas/ You can remove it at the end of the %install section, probably just before %find_lang would be best.
done
[?]: The spec file handles locales properly.
Looks to me like the locale files are all in the right place, but when I changed my system language to French and German, the settings widget was still all in English. This is *probably* a problem upstream, since the version installed from the EGO site doesn't seem to translate either. https://github.com/hedayaty/NetSpeed/issues/55
I have also determined, but still no solution. Upstream is dead ?
[!]: Requires correct, justified where necessary.
Should require GNOME Shell 3.10+, according to metadata.json's list of compatible versions of GNOME. For any GNOME Shell extension, you should also list gnome-shell-extension-common as a dependency.
done
[x]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
There's an open issue about this: https://github.com/hedayaty/NetSpeed/issues/50
added link to ticket.
The patch which adds newer versions of GNOME to the compatibility list should hopefully be taken care of upstream:
done
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/gnome-shell-extension-netspee... SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/gnome-shell-extension-netspee...
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #8 from mgansser@alice.de mgansser@online.de --- Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/gnome-shell-extension-netspee... SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/gnome-shell-extension-netspee...
%changelog * Mon Mar 20 2017 Martin Gansser martinkg@fedoraproject.org - 3.17-0.5.20160806git16a25ec - Add fix_gettext-domain.patch
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #9 from Andrew Toskin andrew@tosk.in --- (In reply to leigh scott from comment #6)
The changelog declares the current version as 3.16
I'd interpreted the version numbers there to be the version of GNOME Shell with which they were working to be compatible. This would be an abuse of the usual CHANGELOG format, but would explain why:
* Each listed releases seems to skip several releases -- not just odd/even releases like the current GNOME convention, but often *four* "minor" releases at a time. * The "Oct 12 2013" release has two different versions, which match the explicitly referenced GNOME Shell versions in that release's description. * A version number of 3.x matching the GNOME scheme conflicts with the version number used in metadata.json which is a single integer, currently in the mid 20s.
...But maybe that's also something upstream should clarify. I've added another comment to this issue:
https://github.com/hedayaty/NetSpeed/issues/52
I'm beginning to have doubts that upstream is ever going to answer, though.
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
Andrew Toskin andrew@tosk.in changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
--- Comment #10 from Andrew Toskin andrew@tosk.in --- Anyway, more review:
Line 17 of the spec: Source1 tag is missing a colon.
You're still including the schemas in the extension's own directory. You don't need this. Spec file line 50:
mkdir -p %{buildroot}%{_datadir}/gnome-shell/extensions/%{uuid}/schemas
Lines 56 and 57:
install -Dp -m 0644 schemas/gschemas.compiled \
%{buildroot}%{_datadir}/gnome-shell/extensions/%{uuid}/schemas/gschemas.compiled
Everything else looks good to me, except for the pending issues on GitHub.
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
Andrew Toskin andrew@tosk.in changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |andrew@tosk.in
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #11 from mgansser@alice.de mgansser@online.de --- (In reply to Andrew Toskin from comment #10)
Anyway, more review:
Line 17 of the spec: Source1 tag is missing a colon.
removed
You're still including the schemas in the extension's own directory. You don't need this. Spec file line 50:
mkdir -p %{buildroot}%{_datadir}/gnome-shell/extensions/%{uuid}/schemas
Lines 56 and 57:
install -Dp -m 0644 schemas/gschemas.compiled \
%{buildroot}%{_datadir}/gnome-shell/extensions/%{uuid}/schemas/gschemas. compiled
removed
Everything else looks good to me, except for the pending issues on GitHub.
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/gnome-shell-extension-netspee... SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/gnome-shell-extension-netspee...
%changelog * Tue Mar 21 2017 Martin Gansser martinkg@fedoraproject.org - 3.17-0.6.20160806git16a25ec - Add missing colon at Source1 tag - Remove schemas in the extension's own directory
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #12 from mgansser@alice.de mgansser@online.de --- (In reply to Andrew Toskin from comment #10)
Anyway, more review:
Line 17 of the spec: Source1 tag is missing a colon.
You're still including the schemas in the extension's own directory. You don't need this. Spec file line 50:
mkdir -p %{buildroot}%{_datadir}/gnome-shell/extensions/%{uuid}/schemas
Lines 56 and 57:
install -Dp -m 0644 schemas/gschemas.compiled \
%{buildroot}%{_datadir}/gnome-shell/extensions/%{uuid}/schemas/gschemas. compiled
Everything else looks good to me, except for the pending issues on GitHub.
I have made your suggestion, but I get the following error message when running the Preferences dialog of gnome-tweak-tool or gnome-shell-extension-prefs:
"GLib.FileError: Failed to open file '/usr/share/gnome-shell/extensions/netspeed@hedayaty.gmail.com/schemas/gschemas.compiled': open() failed: No such file or directory
Stack trace: @/usr/share/gnome-shell/extensions/netspeed@hedayaty.gmail.com/prefs.js:31
Application<._getExtensionPrefsModule@resource:///org/gnome/shell/extensionPrefs/main.js:74 wrapper@resource:///org/gnome/gjs/modules/lang.js:178
Application<._selectExtension@resource:///org/gnome/shell/extensionPrefs/main.js:89 wrapper@resource:///org/gnome/gjs/modules/lang.js:178
Application<._extensionFound/<@resource:///org/gnome/shell/extensionPrefs/main.js:202 main@resource:///org/gnome/shell/extensionPrefs/main.js:377 @<main>:1"
upstream ticket: https://github.com/hedayaty/NetSpeed/issues/56
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #13 from mgansser@alice.de mgansser@online.de --- Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/gnome-shell-extension-netspee... SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/gnome-shell-extension-netspee...
%changelog * Wed Mar 29 2017 Martin Gansser martinkg@fedoraproject.org - 3.17-0.7.20160806git16a25ec - Add netspeed_schema.patch
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #14 from mgansser@alice.de mgansser@online.de --- Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/gnome-shell-extension-netspee... SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/gnome-shell-extension-netspee...
%changelog * Wed Jul 19 2017 Martin Gansser martinkg@fedoraproject.org - 3.17-0.8.20170719git0b769d5 - Update to recent git version 3.17-0.8.20170719git0b769d5
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #15 from Andrew Toskin andrew@tosk.in --- There are a couple important issues still open upstream, but it's nice to see things moving again :)
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #16 from mgansser@alice.de mgansser@online.de --- @Andrew I think we can go ahead with the review.
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/gnome-shell-extension-netspee... SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/gnome-shell-extension-netspee...
%changelog * Wed Feb 07 2018 Martin Gansser martinkg@fedoraproject.org - 3.26-0.1.20180207git02a51b2 - Update to recent git version 3.26-0.1.20180207git02a51b2
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #17 from Andrew Toskin andrew@tosk.in --- I'm pretty sure 3.26 is supposed to be the latest version of GNOME Shell which the extension was tested against. GNOME Shell *extensions* all have a metadata.json file, which includes a "version" property. This is supposed to state the version of the extension itself, and for NetSpeed, that property is 28.
https://github.com/hedayaty/NetSpeed/blob/master/metadata.json
I've tried asking the upstream developer, hedayaty, to git-tag his releases. Or at least to explain to us how to know when the code is stable enough for release. But he doesn't seem to get it, and I'm not sure how else to explain what I'm asking about :/
So, I guess continue with your %{gitdate} and %{shortcommit} Releases. But I'm pretty sure the spec Version should be 28.
I'll double check everything else one last time when that's done.
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #18 from mgansser@alice.de mgansser@online.de --- (In reply to Andrew Toskin from comment #17)
I'm pretty sure 3.26 is supposed to be the latest version of GNOME Shell which the extension was tested against. GNOME Shell *extensions* all have a metadata.json file, which includes a "version" property. This is supposed to state the version of the extension itself, and for NetSpeed, that property is 28.
https://github.com/hedayaty/NetSpeed/blob/master/metadata.json
ok, i will take the version from the metadata file.
I've tried asking the upstream developer, hedayaty, to git-tag his releases. Or at least to explain to us how to know when the code is stable enough for release. But he doesn't seem to get it, and I'm not sure how else to explain what I'm asking about :/
So, I guess continue with your %{gitdate} and %{shortcommit} Releases. But I'm pretty sure the spec Version should be 28.
yes, that's how I see it too.
I'll double check everything else one last time when that's done.
ok
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/gnome-shell-extension-netspee... SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/gnome-shell-extension-netspee...
%changelog * Wed Feb 07 2018 Martin Gansser martinkg@fedoraproject.org - 3.28-0.2.20180208gite3cea60 - Update to recent git version 3.28-0.2.20180208gite3cea60
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
Andrew Toskin andrew@tosk.in changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review+
--- Comment #19 from Andrew Toskin andrew@tosk.in --- I'd meant the version should be *just* 28, because that's what the "version" property in the json file says, not 3.28.
But I won't make you wait for yet another review because that.
Sorry, this has taken so long, but there's only so much you can do when the upstream developer gets busy with other things in their life. At least it's done now, though :)
APPROVED!
The next steps include requesting the creation of the repository for you package in tje Fedora SCM, and requesting all branches for all versions of Fedora and/or EPEL you plan on building for. (master will be created automatically, but you'll need to manually request f27 and f26, and epel7 if you wish to support that release too.)
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers?rd=Pa...
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #20 from mgansser@alice.de mgansser@online.de --- Thanks for the review ! the version is now gnome-shell-extension-netspeed-28-0.2.20180208gite3cea60 hope that's ok now.
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #21 from mgansser@alice.de mgansser@online.de --- creating a new repo fails at the moment ... https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org /thread/IEEBDYWNBMFSPJYAQI5J3YTDLWBAH2RK/
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #22 from Andrew Toskin andrew@tosk.in --- Did you get it working? I don't see netspeed's repo request on Pagure.
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #23 from Andrew Toskin andrew@tosk.in --- Never mind, I found you.
https://pagure.io/releng/fedora-scm-requests/issue/4602
Your Red Hat Bugzilla and Fedora Account System accounts need to use the same email address.
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #24 from MartinKG mgansser@online.de --- I have send a email to the bugzilla-owner@redhat.com with this content:
Does the Bugzilla Admins can merge the two accounts mgansser(at)online.de and mgansser(at)alice.de or disable/delete the mgansser(at)alice.de account.
I received the following message as an answer: Hi Eric,
Ticket URL: PNT0170047
Thank you for contacting the Red Hat PnT Devops team. Your ticket has been opened with the following Priority, Severity, Impact, and Urgency:
Priority: 4 - Normal Severity: 4 - Normal Impact: 4 - Low Urgency: 4 - Low
This request will stay at the current priority and severity until one of our technicians reviews it. Your ticket will have no guarantee of an SLA until it's properly triaged. All email requests come to us without service specifications.
Triaging email requests takes time, and email is not an efficient way to get a hold of us. We have been hard at work creating forms to better filter and triage your requests on the PnT DevOps One Portal (https://pnthelp.redhat.com). Simply search for what you want or peer through our service catalog by clicking "Create a Ticket." This saves us all time, and will ultimately increase the speed at which your request is handled.
As your request is processed, you will receive email notifications with updates.
Regards,
Red Hat PnT DevOps Team
Ref:MSG13543581
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #25 from MartinKG mgansser@online.de --- Hi Andrew,
I want to close the bugzilla ticket and open another ticket, then the new ticket will also contain my new account information, is this ok for you ?
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #26 from Andrew Toskin andrew@tosk.in --- I guess so. Post the link to the new ticket here, and I'll follow you to the new one.
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
--- Comment #27 from MartinKG mgansser@online.de --- Hi Andrew,
I have created a new ticket: https://bugzilla.redhat.com/show_bug.cgi?id=1550396
https://bugzilla.redhat.com/show_bug.cgi?id=1377631
MartinKG mgansser@online.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |WONTFIX Last Closed| |2018-03-02 08:12:52
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com -
Red Hat Bugzilla