[Bug 755510] Review Request: gnome-shell-extension-system-monitor-applet - Gnome shell system monitor extension

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=755510

--- Comment #28 from nicolas.vieville@xxxxxxxxxxxxxxxxxxxx ---
Hello community,

Jeff, thank you for your advice. So here are the new SPEC, SRPMS files and
noarch RPMS packages for testing and modified as possible as you asked for.
Rpmlint output are provided too. Caution, as usual previous version of SRPMS
and RPMS files for Rawhide and F-17 are no longer available on my dropbox
shared folder. RPMS for F-15 and F-16 are still here and can be downloaded
through the URL provided in message #20.

> 1) Remove defattr as it is not required.

Done. Thanks for pointing this I had missed.

> 2) The github downloading stuff looks weird. I'm not sure where the gitsub
> part came from (although I'm also in the process of working this out for
> another package myself). Can you make your Source URL mirror the same format
> used in these other gnome shell extensions?

As you already noticed it in comment #3, this is a real problem. As I'm not the
author of this project, I cannot modify the versioning schema he uses. The only
URL to download the sources of this extension doesn't contain the name of the
file you get when you download it with a web browser. So wget or curl get
confused and store a file named "master" instead of a more complex tar.gz name.

To go further on this point, if you try curl -s --head
https://github.com/paradoxxxzero/gnome-shell-system-monitor-applet/tarball/master
(eg. the download link on the github web page), you can see a new location. And
if you try again curl -s --head
https://nodeload.github.com/paradoxxxzero/gnome-shell-system-monitor-applet/tarball/master
(the new location extracted from the above command), you can see an attachment:
the real tar.gz name containing all the stuff looking weird, including the
gitsub number. The only solution I've found to deal with this was to follow
Fedora guidelines suggestions in including in the spec file all the comments
necessary to understand where to download the source file and what to do with
it.

In such a situation and even in looking at the URLs examples you provided, I'm
a bit puzzled on what to do. If you can show me a satisfactory solution, I
would be glad to apply it.

> 3) The translation stuff looks like it could be greatly simplified using
> find -exec.

Thanks to your advice, my skill in find and xargs command increased, but seeing
the result for the same action as my previous shell script commands, it seems a
bit cryptic, even if it complies with the "oneliner" directive. The biggest
problem was to rename .pot file only if no .po file was present in each lang
directory, and this before making any .mo file. The "if then" statement
couldn't be avoided. I hope these changes meet Fedora recommandations and your
request.

> 4) I think it's policy to not restart gnome shell automatically, so go ahead
> and remove the post scriptlets.

Done!


> You should also show the upstream maintainer this review, just for awareness.

I've already invited the upstream maintainer to follow this review a few months
ago
(https://github.com/paradoxxxzero/gnome-shell-system-monitor-applet/issues/77).
And more later, I submitted some patches to improve translations or some
particular points.

> I'll do a complete (unofficial) package review once all these things are
> completed.

Thanks in advance! I hope the modifications I've made meet your requests.

To be complet on this subject, I've also commented the patch directives in the
SPEC file as recommended in Fedora Packaging Guidelines.

Rawhide Spec URL:
http://dl.dropbox.com/u/25699833/Fedora/Rawhide/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet.spec

Rawhide SRPM URL:
http://dl.dropbox.com/u/25699833/Fedora/Rawhide/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet-2.0b1-0.1.git3117df5.fc18.src.rpm

Rawhide RPM URL:
http://dl.dropbox.com/u/25699833/Fedora/Rawhide/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet-2.0b1-0.1.git3117df5.fc18.noarch.rpm

F-17 RPM URL:
http://dl.dropbox.com/u/25699833/Fedora/F-17/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet-2.0b1-0.1.git3117df5.fc17.noarch.rpm

Project URL: https://github.com/paradoxxxzero/gnome-shell-system-monitor-applet

rpmlint for Rawhide

$ rpmlint gnome-shell-extension-system-monitor-applet.spec 
gnome-shell-extension-system-monitor-applet.spec: W: invalid-url Source0:
paradoxxxzero-gnome-shell-system-monitor-applet-2.0b1-123-g3117df5.tar.gz
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

$ rpmlint
gnome-shell-extension-system-monitor-applet-2.0b1-0.1.git3117df5.fc18.src.rpm
gnome-shell-extension-system-monitor-applet.src: W: invalid-url Source0:
paradoxxxzero-gnome-shell-system-monitor-applet-2.0b1-123-g3117df5.tar.gz
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Thank you again and happy testing!

Cordially,


-- 
NVieville

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]