Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: dmenu - Dynamic X menu
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Summary: Review Request: dmenu - Dynamic X menu Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: low Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: appolito2@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu.spec SRPM URL: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu-3.9-1.src.rpm
Description:
Dynamic menu is a generic menu for X, originally designed for dwm. It manages huge amounts (up to 10.000 and more) of user defined menu items efficiently.
rpmlint output: 3 packages and 1 specfiles checked; 0 errors, 0 warnings. mock build log: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/build.log
Please note that this is my first fedora package and I am seeking a sponsor.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Jan Blazek appolito2@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Suravee Suthikulpanit suravee.suthikulpanit@amd.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |suravee.suthikulpanit@amd.c | |om
--- Comment #1 from Suravee Suthikulpanit suravee.suthikulpanit@amd.com 2009-02-18 16:02:50 EDT --- This is an un-official pre-review.
- May I suggest append "%{dist}" at the end of release string.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #2 from Jan Blazek appolito2@gmail.com 2009-02-21 03:29:17 EDT --- (In reply to comment #1)
This is an un-official pre-review.
- May I suggest append "%{dist}" at the end of release string.
Thank you. It's repaired now.
New SPEC: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu.spec New SRPM: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu-3.9-2.fc10.src.r...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Till Maas opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |opensource@till.name
--- Comment #3 from Till Maas opensource@till.name 2009-03-31 07:24:57 EDT --- Imho it is better to use a patch instead of the sed commands and modify the config.mk in a way that you can pass the %{optflags} on the commandline. This is easier to read and can be submitted upstream.
I also recommend to remove the "@"-signs from the Makefile to see which commands are actually run, which helps to verify in the build.log, whether or not the %optflags are really honoured: sed -i 's/\t@/\t/' Makefile
Btw. strncat is not properly used in dmenu.c, whis is shown in a compiler warning: CC dmenu.c In function 'strncat', inlined from 'kpress' at dmenu.c:400, inlined from 'run' at dmenu.c:562, inlined from 'main' at dmenu.c:725: /usr/include/bits/string3.h:153: warning: call to __builtin___strncat_chk might overflow destination buffer Here is a patch by me: http://till.fedorapeople.org/files/dmenu-3.9-strncat.patch There is no need to submit it upstream, because they removed the strncat.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #4 from Jan Blazek appolito2@gmail.com 2009-04-08 08:50:01 EDT --- Thanks for suggestions. New SPEC: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu.spec New SRPM: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu-3.9-3.fc10.src.r... New mock log: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/build.log rpmlint doesn't have any complaints
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #5 from Jan Blazek appolito2@gmail.com 2009-04-08 08:52:17 EDT --- I've send the patch to upstream (mailing list) http://lists.suckless.org/dwm/0904/7801.html
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Jens Petersen petersen@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |petersen@redhat.com
--- Comment #6 from Jens Petersen petersen@redhat.com 2009-04-19 06:14:26 EDT --- 4.0 was released apparently.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #7 from Jens Petersen petersen@redhat.com 2009-04-19 06:29:46 EDT --- Please don't use %{version} in the patch filenames since it breaks when bumping version.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #8 from Jan Blazek appolito2@gmail.com 2009-04-19 08:28:24 EDT --- Updated to 4.0 and corrected the patch filename.
New SPEC: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu.spec New SRPM: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu-4.0-1.fc10.src.r... New mock log: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/build.log
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #9 from Till Maas opensource@till.name 2009-04-19 15:28:10 EDT --- Sorry, I meant to reply earlier. The LDFLAGS now contain "-s", which strips the binaries. This should also be changed, maybe with advancing the Makefile patch to also allow to remove the -s flag.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #10 from Jan Blazek appolito2@gmail.com 2009-04-19 17:43:23 EDT --- (In reply to comment #9)
Sorry, I meant to reply earlier. The LDFLAGS now contain "-s", which strips the binaries. This should also be changed, maybe with advancing the Makefile patch to also allow to remove the -s flag.
Is it OK to make one patch for both LDFLAGS and OPTFLAGS?
New SPEC: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu.spec New SRPM: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu-4.0-2.fc10.src.r... New mock log: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/build.log
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Chess Griffin chess@chessgriffin.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |chess@chessgriffin.com
--- Comment #11 from Chess Griffin chess@chessgriffin.com 2009-06-23 23:03:52 EDT --- Hello-
I cannot give you an official review, but here are a few minor nits:
You have "Source:" and "Patch0:" (one has 0 and one does not). The documentation[1][2] references "Source0:" even there is only one source download. Either way, they should be consistent, so you may want to change "Source:" to "Source0:".
Additionally, (and this is a very minor quibble), you have some parts of the spec file with one blank line between sections and other parts with two. Personally, I would stick with one style or the other. It just looks cleaner.
FWIW, I would love to see this package in Fedora. :-)
[1] http://fedoraproject.org/wiki/PackageMaintainers/CreatingPackageHowTo [2] http://fedoraproject.org/wiki/Packaging/SourceURL
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Jan Blazek jblazek@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jblazek@redhat.com
--- Comment #12 from Jan Blazek jblazek@redhat.com 2009-06-29 16:14:01 EDT --- (In reply to comment #11) Hi,
thank you your suggestions. You're right about the blank line consistency and Source vs Source0, although it doesn't affect the functionality.
New SPEC: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu.spec New SRPM: http://www.stud.fit.vutbr.cz/~xblaze17/packages/dmenu/dmenu-4.0-3.fc10.src.r...
PS: I'd also love to see this package in Fedora :-)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Rahul Sundaram sundaram@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |sundaram@redhat.com AssignedTo|nobody@fedoraproject.org |sundaram@redhat.com
--- Comment #13 from Rahul Sundaram sundaram@redhat.com 2009-08-13 11:27:12 EDT ---
Remove the buildroot definition and you dont need to clean it in %install section anymore.
https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
Add a comment explaining what the patch does and it's upstream status
https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Rahul Sundaram sundaram@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #14 from Till Maas opensource@till.name 2009-08-13 11:40:32 EDT --- (In reply to comment #13)
Remove the buildroot definition and you dont need to clean it in %install section anymore.
https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
To be more precise: It's is only not needed, if the package is not going into EPEL 5 or 4.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #15 from Rahul Sundaram sundaram@redhat.com 2009-08-14 07:17:41 EDT --- Right but then I would suggest that you keep them only in the EL branches and remove unnecessary cruft from the Fedora branches but that's left to the maintainer.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Rahul Sundaram sundaram@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|sundaram@redhat.com |nobody@fedoraproject.org
--- Comment #16 from Rahul Sundaram sundaram@redhat.com 2009-08-14 07:20:39 EDT --- Didn't realize this one requires a sponsor. Taking myself off as the reviewer although I can review it unofficially and make it easy for the sponsor.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Jussi Lehtola jussi.lehtola@iki.fi changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jussi.lehtola@iki.fi
--- Comment #17 from Jussi Lehtola jussi.lehtola@iki.fi 2009-08-14 08:02:49 EDT --- I don't agree. I think keeping the BuildRoot stuff doesn't hurt anything: it's just a few lines, and it avoids using conditionals that are a lot easier to get wrong. Having one and only spec file for the different branches is a plus, also.
**
- The %description line fits on two lines. No need to use four.
- Any reason why you are not using SMP make? If it doesn't work, please document it.
- Your patch doesn't have a comment. Add one explaining what it does and why it is necessary.
- I think you are missing a Requires: on the package that provides the font dmenu uses.
**
I am a sponsor, so I can sponsor you if necessary. You just need to do a few informal reviews first.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #18 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2009-09-10 13:23:46 EDT --- Please assign this bug to someone who is actually reviewing this review ticket _formally_ , which must be a sponsor and set fedora-cvs flag properly.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Till Maas opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |
--- Comment #19 from Till Maas opensource@till.name 2009-09-10 14:43:04 EDT --- (In reply to comment #18)
Please assign this bug to someone who is actually reviewing this review ticket _formally_ , which must be a sponsor and set fedora-cvs flag properly.
Nobody is reviewing this, therefore I reset thes fedora-review flag, which you probably meant with fedora-cvs flag.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #20 from Till Maas opensource@till.name 2009-09-19 08:35:23 EDT --- Jan, did you perform some inofficial reviews, like one is advised on the how to get sponsored wiki page[0]?
[0] https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group#...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Jason Woofenden jason310@herkamire.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jason310@herkamire.com
--- Comment #21 from Jason Woofenden jason310@herkamire.com 2009-10-03 16:18:59 EDT --- Jan,
I'm getting a 403 error when I try to download your srpm :(
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #22 from Jan Blazek jblazek@redhat.com 2009-10-03 16:36:17 EDT --- (In reply to comment #21)
Jan,
I'm getting a 403 error when I try to download your srpm :(
Sorry, that account doesn't work anymore. I've moved it to new url and added bitmap-fonts to Requires.
http://appolito.ic.cz/packages/dmenu-4.0-4.fc11.src.rpm http://appolito.ic.cz/packages/dmenu.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #23 from Till Maas opensource@till.name 2009-10-14 13:54:19 EDT --- Jan, I still cannot find any inofficial reviews from you, like I stated in comment:20. I highly doubt it, that someone will move forward and sponsor you, without you doing these inofficial reviews, like it is recommended on the "how to get sponsored wiki page"[0].
[0] https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group#...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cassmodiah@fedoraproject.or | |g
--- Comment #24 from Jason Tibbitts tibbs@math.uh.edu 2009-10-15 18:21:14 EDT --- *** Bug 529253 has been marked as a duplicate of this bug. ***
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #25 from Simon Wesp cassmodiah@fedoraproject.org 2009-12-11 07:32:42 EDT --- What's the current state of this bug?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #26 from Jan Blazek appolito2@gmail.com 2009-12-11 07:46:34 EDT --- Sorry, but I'm quite busy ATM. If someone wants to pick up this package feel free to do it.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
--- Comment #27 from Simon Wesp cassmodiah@fedoraproject.org 2009-12-11 07:49:45 EDT --- Then please close this bug and i will open a new one.... I need this pkg for my i3 desktop
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=485638
Jan Blazek appolito2@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Blocks|177841(FE-NEEDSPONSOR) | Resolution| |NOTABUG
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla