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=510734 --- Comment #56 from Christian Krause <c...@plauener.de> 2009-08-27 18:57:00 EDT --- Thanks for the new package. There are just some smaller cosmetic changes left. (In reply to comment #53) > > * specfile in American English and legible: TODO > > - indentation: I know this may sound like nitpicking, however I'm still > > convinced that the spec files should be written in such a way that it can be > > easily read by any other user with any program. It should not be necessary > > to > > guess the tab width for each spec file. ;-) I think using some kind of > > standard > > for things like this will certainly help that all Fedora package maintainers > > can easily work together. > There was discussion on this theme - > http://thread.gmane.org/gmane.linux.redhat.fedora.extras.packaging/6214/focus=6224 > And accordingly it proposed draft guidelines change - > https://fedoraproject.org/wiki/PackagingDrafts/Tabs > > So, there many people argue with your arguments. So, I think until it is only > draft and FESCO do not approve that, it can't be as required item. Yes, I've also read this thread. However, it is for sure not wrong to use a standard tab width. And you would do nearly all other packagers the favor that they could easily read your spec file with a proper formatting. The alternative would be to use just spaces. This would work for everyone and since there is usually not that many changes in a spec file, it should not have that much overhead. Would you agree on this solution? > > - typo: pathes -> paths > > - wording: IMHO "performant" isn't an Enlish word, probably just use "fast" > Sure? http://dictionary.reference.com/browse/performant Not any more. ;-) I did some more research: In the mentioned URL the meaning of "performant" is described as "a performer" but not as an adjective with the meaning of "fast". However, just searching for "performant" with google revealed lots of people debating about it. ;-) Even if it is not in the standard dictionaries, it can be found here: http://www.urbandictionary.com/define.php?term=Performant > But if you want, I replace it by productive. Is it ok? Either way is fine with me by now. ;-) > > - spelling: "prebuild clients" -> "prebuilt clients" > > - spelling: "acording" -> "according" > > - spelling: "macroses" -> "macros" > There I don't known (replaced as you say). Macros have not multiple form at > all? Yes, "macros" is in widespread use. > > - indentation in %prep: > > The small code snippet to do the conversion into UTF-8 is not indented > > correctly. The begin ("for file....") and the end ("done") of the for loop > > should be one tab more to the left than the body of the for loop. > Again... > I'm change it as you like see it for only do not start new long flame-war. > But this is very similar to tab-width space. In Fedora we even not have (as I > known) some recommended "coding standards" or similar > documents/guidelines/policies. So, than it can't be error at all! I think basic indentation is a well-accepted coding standard for nearly all languages. Especially the inner blocks of constructs like "if () then ... else ..." or "for" loops should be indented one step more than the surrounding code. Here are the missing minor pieces: 1. tab width: it would really be nice if you could use either a tab width of 8 or just convert the spec file to spaces 2. According to Tom it is not necessary to re-package the tarball. However, he suggests that the pre-built binaries are deleted in the %prep section. My suggestion: - add "find -name '*.jar' -exec rm {} \;" to the end of the %prep section - add the attached patch to prevent that "make" will enter the "classes" directory 3. Please make sure that the comments in the spec file and the %changelog entries don't exceed 80 chars per line. Sure, that's also not strictly required but nearly all spec files do it this way. ;-) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@redhat.com http://www.redhat.com/mailman/listinfo/fedora-package-review
