Michal, et.al. Let me first tell you: I understand your frustration. And the fact that somebody with a project maintainer role oversteps the package maintainer, giving him 5 hours of notice period, is bad behavior. I stronlgy encourage the two of you to talk this out directly. As to the points you list in the 'change set', I only want to give some small words there (from a opensuse-review team perspective - but it's my own, which I hope the rest of the reviewer team shares). On Tue, 2016-02-02 at 00:10 +0100, Michal Kubecek wrote:
- replace xz compressed tarball by the original gzipped one For years I've been told by OBS that I should repack gzipped tarballs to save resources. It's not true any more? OK, it allows for checking the signature so it makes sense.
Most of the distro packages switched to using the prisitine upstream tarball quite a while ago, especially for checking signatures, and, as I will explain in pioint 3
- make BuildRoot specification unconditional For years I've been told that it is obsolete. The only reason I keep it is to allow build for SLE11. Why forcing it on distributions that don't need it?
It is indeed needed for SLE11; I have no strong pro or contra if it should be provided for non-SLE11 builds, but it's purely used during the build phase, so does not impact the resulting package. So it comes down to: do you prefer a condition without added value in your spec file? Or do you prefer a 'plain' readable one with as few churn as possible. As you stated: it's your package - your choice.
From a Factory Reviewer PoV: no objection either way. YOUR CHOICE!
- replace source files by full URL I don't like this change. More precisely: I hate it.
It has one big advantage: trust! Every package submitted to openSUSE:Factory, the review bot actually downloads the tarball from the URL in the Source line and verifies that the submitted one actually IS The one it's supposed to be. This avoids that a packager can inject anything into a tarball (as you can imagine, there are not sufficient resources to manually review all the submitted code - we need to be able to trust one another. The URL as Source allows to automate it)/
- add "-q" to %setup Why? No idea.
Again, personal choice. Factory Reviewer accepts either way. The most notable thing '-q' does here is not list the entire content of the extracted tarball... so making the build log 'neater'
- add "V=1" to make Why? To make build warnings harder to see?
A constant battle - I know that one. I myself consider V=1 great for debug perpose... as long as gcc's warnings are there if it's not specified, I don't enable it.
- break configure line into two If it needed breaking, I would do so. This one had 32 characters... Seriously?
That's really a strange one to even think to enforce. Certainly, no packaging guideline exists around this topic. And obviously, there are cases it makes sense to split the lines, there are others it's useless. Your case seems to be the latter.
- replace ${RPM_BUILD_ROOT} with %{buildroot} The only rule I have ever seen about this was there is no rule.
There is a small difference in the expansion of the variable: does bash do it (${}) or does rpm do it %{}. The value is the same. style-wise, many prefer the lower case writing style (spec-cleaner does change this one to).
- add %{?_smp_mflags} also to make install Well, why not. But definitely not _after_ the argument.
Parallel installation - how big is that package :) I doubt it makes much difference (but then: the latest version of spec-cleaner does this automatically IIRC - so the submitter might not have done this explicitly)
- add LICENSE to the package OK, why not.
Actually mandatory to pass legal review. Which, by the way, is what the current submission to openSUSE:Factory is pending at: https://build.ope nsuse.org/request/show/356511 (not staging per se.... just that it can only move out of staging once all review steps are handled)
- %files: expand wildcards to explicit lists Some people apparently prefer package build to fail whenever a file is added or renamed. I don't force them not to do so in their packages, could they respect my preferences in mine? Apparently not.
Indeed - personal style to chose. From experience, I use explicit lists when it's about a lot of plugin-style files that get auto-enabled based on deps. Like this, I see when a dep goes missing: the build fails.
- reorder tags in specfile header I'm confused. The previous order was what spec cleaner did. Wasn't it supposed to be The Right Order(TM)? And what happens next time spec cleaner service is run? Is it going to change the order back, masking real changes?
Sometimes, new version of spec cleaner might do it different. Only solution is for us all to run the same version of the tool.
Summary: lot of meaningless churn, one useful change, two I dislike and two (or two and a half) I really dislike. And a bad feeling that while I took responsibility for the package, I apparently have no say in what should its specfile look like. Don't understand me wrong: I have no problem with reasonable rules, even if they are quite strict (like the tagging policy in our kernel packages or strict coding style policy in kernel networking). But here I feel this crosses a line of harassment.
I agree that there are quite some questionable changes forced into your package. As an active maintainer, I actually suggest you to REVERT the change and implement the changes you like on your own. Don't play around: You are the maintainer of the package, you care for it. Show it - revert.
OK, I'll learn to live with it. I'll bite the bullet with packages like
Please don't just learn to live with it! Your approach to bring this up here is absolutely perfect and I hope peopple take this constructive critic on when they do changes. Do not step on each others feet, but collaborate!
firebird or twinkle which are kind of "heart thing". But next time I stumble upon a package that might be useful, I'll think twice before submitting it to Factory. Had I known what was going to happen, well, there would be one less package in Factory (assuming tcpreplay is going to make it). And just today, I got a reaction like "Been there. I know exactly what you are talking about."
In the hopes not to lose your spirit of helping others with your work: please re-consider... your packages are welcome in the distribution.
Sure, you may say that it's my fault I don't understand openSUSE high standards. You might even say openSUSE is not interested in packagers who don't understand its high standards. But you should understand in the end it's always packager's choice if he is going to submit the package to Factory or not. And remember, to "scratch an itch", putting it into a home project is enough in most cases; submitting to Factory is not about "scratching an itch", it's about helping others; things like this work as an active discouragement.
Whereas it's true that we have packaging guidelines, and some rules we enforce stricter than others: ever tried to submit something to Debian? :) Jokes aside: our guidelines are there to 'guide' - some of the rules are strictly enforced (most of them by a bot!), the review team tries to 'see more common errors' and things the bot might mis-understand (like: static libs: they are a no-go, except when done properly and documented and fully considered - so the bot can't know)
Think about it next time you ask yourself why so many packages stay in home or devel project without ever being submitted to Factory. Or, even better, think about it next time you feel urge to completely rearrange a package you are not maintaining (and not going to) just to force your aesthetic criteria on it (and its maintainer).
Thank you Michal for bringing this up! I agree: aestethics are not a reason for a decline (even though I can assure you that a spec that looks ugly and is more cryptic to read than needed will likely make it to the 'end of the review queue'). For me, the important message in this entire thing is to ALL contributors: contribute to any package you want - be active, but if there is a person dedicated to maintain it (and this person is still active): let this person have a word if he/she likes the change. There are 'teams' building around certain packages, where people talk together and come to a common view on how they want to maintain it - so they jointly do the work, sometimes with, sometimes without consulting. There are even larger projects where NO maintainer in the project is supposed to accept his own change! Strict 4-eye peer review even before stuff moves to Factory. This is not mandatory in OBS - but if the active maintainer so wishes, it is to be respected. Cheers, Dominqiue -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org