Spec URL: https://jorti.fedorapeople.org/zram-swap/zram-swap.spec SRPM URL: https://jorti.fedorapeople.org/zram-swap/zram-swap-1.0.2-1.fc21.src.rpm Description: zram compresses swap partitions into RAM for performance Fedora Account System Username: jorti
Taking this review
Greetings, a few comments: 1. Why not to push latest SPEC to your pull-request? I mean this: https://github.com/mystilleef/FedoraZram/pull/12/files It could be merged and then you can use upstream SOURCE. 2. You could easily remove empty %build from your SPEC. 3. Package name is misleading. It's not related to zswap so you should probably rename it to something like zramctl. 4. Why you don't want to include zramstat? It was very useful util. 5. Why you decided to use manual install instead of %makeinstall? Thanks! Please note that this is informal review.
(In reply to Vladimir Stackov from comment #2) > Greetings, > > a few comments: > > 1. Why not to push latest SPEC to your pull-request? > I mean this: https://github.com/mystilleef/FedoraZram/pull/12/files > It could be merged and then you can use upstream SOURCE. That is another repo. I took the scripts from https://lists.fedoraproject.org/pipermail/devel/2014-November/204684.html > > 2. You could easily remove empty %build from your SPEC. rpmlint warns against that > > 3. Package name is misleading. It's not related to zswap so you should > probably rename it to something like zramctl. The script creates a swap on zram, so I think zram-swap fits quite well. > > 4. Why you don't want to include zramstat? It was very useful util. I see that script is from https://github.com/mystilleef/FedoraZram , I could merge it, but we are talking about other repo again. > > 5. Why you decided to use manual install instead of %makeinstall? There is no makefile
(In reply to Juan Orti from comment #3) > (In reply to Vladimir Stackov from comment #2) > > Greetings, > > > > a few comments: > > > > 1. Why not to push latest SPEC to your pull-request? > > I mean this: https://github.com/mystilleef/FedoraZram/pull/12/files > > It could be merged and then you can use upstream SOURCE. > > That is another repo. I took the scripts from > https://lists.fedoraproject.org/pipermail/devel/2014-November/204684.html So maby you should follow https://fedoraproject.org/wiki/Staying_close_to_upstream_projects? lists.fedoraproject.org isn't upstream. > > > > > 2. You could easily remove empty %build from your SPEC. > > rpmlint warns against that Ok, it's all up to you :) > > > > > 4. Why you don't want to include zramstat? It was very useful util. > > I see that script is from https://github.com/mystilleef/FedoraZram , I could > merge it, but we are talking about other repo again. What do you mean when you say "other repo"? It was upstream. > > > > > 5. Why you decided to use manual install instead of %makeinstall? > > There is no makefile Then it's your chance to use upstream repo :) https://github.com/mystilleef/FedoraZram/blob/master/Makefile
The URL seems wrong, and there is no mention of the license anywhere in the source. I'd indeed recommend using the mystilleef repo as your source
Ok, I accept to use mystilleef as the source. I have submitted a new review request at https://bugzilla.redhat.com/show_bug.cgi?id=1179835 *** This bug has been marked as a duplicate of bug 1179835 ***