Bug 207896
Summary: | Review Request: astyle - Source code formatter | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Mary Ellen Foster <mefoster> | ||||||||||
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> | ||||||||||
Status: | CLOSED ERRATA | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||||||
Severity: | medium | Docs Contact: | |||||||||||
Priority: | medium | ||||||||||||
Version: | rawhide | CC: | gnome, haircut, kevin, mtasaka | ||||||||||
Target Milestone: | --- | Flags: | mtasaka:
fedora-review+
kevin: fedora-cvs+ |
||||||||||
Target Release: | --- | ||||||||||||
Hardware: | All | ||||||||||||
OS: | Linux | ||||||||||||
Whiteboard: | |||||||||||||
Fixed In Version: | 1.21-5.fc7 | Doc Type: | Bug Fix | ||||||||||
Doc Text: | Story Points: | --- | |||||||||||
Clone Of: | Environment: | ||||||||||||
Last Closed: | 2007-07-11 15:20:12 UTC | Type: | --- | ||||||||||
Regression: | --- | Mount Type: | --- | ||||||||||
Documentation: | --- | CRM: | |||||||||||
Verified Versions: | Category: | --- | |||||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||||
Embargoed: | |||||||||||||
Attachments: |
|
Description
Mary Ellen Foster
2006-09-25 08:11:02 UTC
p.s. -- Just heard back from the developer, and he's fine with this being in Extras. Ping? (Also adding FE-NEW & FE-SPONSOR so someone can find this package) Created attachment 154591 [details]
specfile for v1.20.2
I needed a specfile for 1.20.2 (the current release), so I made one. Hope this
helps. The Makefile patch no longer works, so I removed it.
Hey Mary. Are you still interested in submitting this package? If so could you update to the latest version and post an updated spec/src.rpm link? You may also want to look at: http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored If I don't hear from you in 1 week I will go ahead and close this submission. I am definitely still interested in packaging this, but I'll be travelling until next Sunday (10 June). I'll update the package as soon as I get back. Thanks for responding! :) Okay, I've uploaded the latest spec and srpm to: http://www6.informatik.tu-muenchen.de/~foster/extras/astyle.spec http://www6.informatik.tu-muenchen.de/~foster/extras/astyle-1.20.2-1.src.rpm The spec is basically the revised one sent by Adam Monsen -- thanks! :) This package will need some pretty intensive love, because its Makefile (build/Makefile) doesn't really fit into rpm's demands. MUSTFIX: * Package doesn't acknowledge RPM_OPT_FLAGS The cause is before-mentioned Makefile. You need to find a way to correctly pass RPM_OPT_FLAGS to CFLAGS of this package (More on this below). * The Makefile strips the installed binary (/usr/bin/astyle) Packages are not supposed to strip binaries. rpm needs the unstripped binaries to generate debuginfos from them and strips binaries itself after having generated the debuginfos * The spec should use %{_bindir} instead of /usr/bin * BuildRequires: gcc-c++ libstdc++ Putting them into BuildRequires is frowned upon, because these are defaults components of the buildsystem. Please remove. Wrt. build/Makefile: Frankly speaking, build/Makefile is in a shape, I'd seriously recommend not to use it, but to manually compile astyle from inside of the spec. This approach would be feasible if you don't plan to also ship the astyle libs. If you plan to do so, patching build/Makefile would be an appropriate approach. Created attachment 156965 [details]
patch against 1.20.2-1.spec
This patch contains changes implementing what I wrote in previous comment.
Thanks for the suggestions; I've incorporated them into the spec (and also added a %{dist} to the version -- is that right?) New stuff here: http://www6.informatik.tu-muenchen.de/~foster/extras/astyle.spec http://www6.informatik.tu-muenchen.de/~foster/extras/astyle-1.20.2-2.fc7.src.rpm Created attachment 157246 [details]
Mock Build Log
Created attachment 157247 [details]
Mock Root Log
General Review --> MUSTS: 1) RPMLINT: rpmlint produces no output - PASS 2) NAMING GUIDELINES: *no underscores in name (since the source contains underscores you could use underscores if you wanted, do you want to? If downstream contains them, which it does, it would be good to be consistent. ) - NEEDINFO *spec file name correct - PASS *package version matches source and correct - PASS *release number correct - PASS *%{?dist} used properly - PASS *package not case sensitive - PASS *no renaming/replacing existing packages - PASS *no subpackages included - NEEDINFO -- Cannot find astyle library files? -- Are the libs he referred to standard C++ includes? -- Are the libs source files in src/ ? -- It would probably be best to include the libs in another pkg. *no addon packages - PASS 3) PACKAGING GUIDELINES: *licensing incorrect - CHANGE -- astyle is licensed under the LGPL, not GPL -- please correct License: -- LGPL documentation included as license.html document *no known patents - NA *not an emulator - NA *no binary firmware - NA *inclusion of libs and/or binaries - NEEDINFO (see subpackages) *pkg from scratch matches minimal spec except %configure - NEEDINFO -- make %{?_smp_mflags} --> should be included if possible -- because the pkg is compiled manually maybe a g++ switch? -- g++ -o astyle $RPM_OPT_FLAGS src/*.cpp could be? - NOT SURE IF NEEDED It's a single (and small) source file so you shouldn't need it I'd think. -- I don't believe %configure is required - NEEDINFO *not modifying an existing spec - NA *filesystem layout - PASS *rpmlint - PASS *changelog - CHANGE -- should remove the last changelog comment about a different version -- if another pkg exists with that version number then you should put the comments in its' spec file -- comment for initial version should match version of the pkg and still exist so history of the pkg is maintained *buildroot - PASS *requires - *prereq - NA *file dependencies - NA *buildrequires - NEEDINFO -- mock produces the following during the debug ... cpio: astyle/<built-in>: No such file or directory -- I am not sure if the above "error" is relevant. -- I've attached the mock build log. -- builds cleanly in mock. *rpmdev-rmdevelrpms - NA *summary and description - PASS *encoding - PASS *non-ascii filenames - NA *documentation - PASS -- It would be nice to have a brief manpage for this pkg that the maintainers would incorporate. *compiler flags - UNKNOWN *debuginfo pkgs - PASS *static linked libraries/executables - NA *libraries - NEEDINFO -- I think this package might need to be split apart into a libraries pkg and a program pkg. I'm not totally sure though, since I'm somewhat "new." Please read up on packaging guidelines to determine the best way to get around the way the makefile is structured. (http://fedoraproject.org/wiki/Packaging/Guidelines) *duplication of sys libs - NA *rpath and removing rpath - NA *configuration file - NA *init scripts - NA *desktop file - NA *macros - PASS *locale files - NA *parallel make - NEEDINFO -- because this pkg uses g++ instead of make in the build section and doesn't have a configure section I don't know how to address this "nice to have." Anyone have an idea? *timestamps - PASS *scriptlets - NA *conditional dependencies - NA *relocatable pkg - NA *code vs. content - PASS *file and dir ownership - PASS *web app - NA *confilcts - NA 4) LICENSING: *need to correct the License: field - CHANGE *other than the problem above - PASS 5) LICENSE FIELD: *see #4 - CHANGE 6) DOC LICENSE: included in proper dir - PASS 7) SPEC FILE IN ENGLISH: - PASS 8) SPEC FILE LEGIBLE: - PASS 9) SOURCES MATCH UPSTREAM: need to replace last comment - PASS 10) COMPILES/BUILDS: built, and installed on my system (pkg runs) - PASS 11) EXCLUDEARCH: are there any arches it doesn't build for? I assume all are fine since the comments state any arch that has g++. - NEEDINFO 12) BUILDREQUIRES: none - PASS 13) LOCALES: none - PASS 14) SHARED LIBS: I think you should strip out the libs and put them in another pkg and change the makefile, but I'm new so a more experienced reviewer should chime in please... - NEEDINFO 15) RELOCATABLE PKG: n/a - NA 16) DIR OWNERSHIP: - PASS 17) DUPLICATE FILES: none - PASS 18) PERMS: - PASS 19) CLEAN SECTION: - PASS 20) CONSISTENT MACROS: - PASS 21) CODE/CONTENT: - PASS 22) LARGE DOCS: n/a - NA 23) MISSING DOCS: moved docs from docdir to home dir and binary functioned properly - PASS 24) HEADER FILES: there is a header in src but I don't think it needs to g into a -devel pkg - NEEDINFO (from a pkg reviewer) 25) STATIC LIBS: again, needinfo - NEEDINFO 26) PKGCONFIG: n/a - NA 27) LIBS w/ SUFFIXES: again, see #25 - NEEDINFO 28) DEVEL PKG: n/a - NA 29) LIBTOOL ARCHIVES: n/a - NA 30) GUI APP DESKTOP FILE: n/a - NA 31) OWN NON-SPECIFIC DIRS/FILES: no this pkg doesn't - PASS 32) REMOVE BUILDROOT IN INSTALL: - PASS 33) UTF-8 FILENAMES: - PASS SHOULDS: 1) Includes LGPL license text. - PASS 2) Description short and concise but no none English translations. This is up to the maintainer if they desire to have this included. - NEEDINFO 3) The package builds in mock with one potential issue: - NEEDINFO *mock produces the following during the debug stage ... cpio: astyle/<built-in>: No such file or directory *I'm not certain this is even an issue b/c the debuginfo pkgs build fine. 4) Package runs but I haven't tested the functionality further than 'astyle --help'. - PASS 5) Scriptlets are not used. - NA 6) No subpackages. There might be a lib pkg generated but again, that's a topic up for discussion with more experienced pkg reviewers. - NEEDINFO 7) Pkgconfig not used. - NA 8) No other known file dependencies other than the libs. - NEEDINFO (In reply to comment #12) > General Review --> Well, If you want to do the review of this style, please make the summary of the review so that everyone (including submitter) can read your review easily. > 2) NAMING GUIDELINES: > *no underscores in name (since the source contains underscores you could > use underscores if you wanted, do you want to? But the name "astyle" has no underscore.. I don't know what is the problem with the name of this pkgname? > *no subpackages included - NEEDINFO > -- It would probably be best to include the libs in another pkg. This can be left to how the submittter judges. > -- astyle is licensed under the LGPL, not GPL > *pkg from scratch matches minimal spec except %configure - NEEDINFO For this package this can be ignored IMO +1 for Ralf's comment > *rpmlint - PASS Umm??? Don't pass (see below) > *changelog - CHANGE > -- should remove the last changelog comment about a different version Why? > -- if another pkg exists with that version number then you should put > the comments in its' spec file > -- comment for initial version should match version of the pkg and > still exist so history of the pkg is maintained Currently I don't understand what you want to say here > -- mock produces the following during the debug ... > cpio: astyle/<built-in>: No such file or directory Can be ingored > *debuginfo pkgs - PASS Actually, don't pass (related with rpmlint - see below) > *libraries - NEEDINFO > -- I think this package might need to be split apart into a libraries > pkg and a program pkg. IMO this is not needed for this package. > *parallel make - NEEDINFO This is not needed for this package (g++ *.cpp meets the demand) > 4) LICENSING: > *need to correct the License: field - CHANGE * For rpmlint: ----------------------------------------------------------- E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/astyle_main.cpp W: astyle-debuginfo spurious-executable-perm /usr/src/debug/astyle/src/astyle.h E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASEnhancer.cpp E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASResource.cpp E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASFormatter.cpp E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASBeautifier.cpp ------------------------------------------------------------- - All of these are permission issues. Please fix these. * Macros - /usr/bin/install <- use macros for /usr/bin. * Documentation - Please check if "install.html" is needed for %doc. This seems to be needed for people who want to rebuild this package by themselves and does not seem to be needed for rpm users. (In reply to comment #13) > (In reply to comment #12) > > General Review --> > Well, If you want to do the review of this style, please make > the summary of the review so that everyone (including submitter) can > read your review easily. FWIW: I find this style of review unreadable and not to be actually helpful. > General Review --> >Well, If you want to do the review of this style, please make >the summary of the review so that everyone (including submitter) can >read your review easily. Hmmm... I thought I was being very thorough in my review. I followed every aspect of the review process to the "T," but from now on will not post everything at once. > 2) NAMING GUIDELINES: > *no underscores in name (since the source contains underscores you could > use underscores if you wanted, do you want to? > But the name "astyle" has no underscore.. I don't know what > is the problem with the name of this pkgname? There aren't any underscores in the name hence the comment "no underscores in the name...". However, the source package has underscores so I simply stated she could, if she wanted, use them because it matches one of the listed exceptions to the rule. > *no subpackages included - NEEDINFO > -- It would probably be best to include the libs in another pkg. > This can be left to how the submittter judges. Great! Thank you for answering my question, as I wasn't sure about this one. > -- astyle is licensed under the LGPL, not GPL Yes. As I highlighted she needs to correct this portion. > *pkg from scratch matches minimal spec except %configure - NEEDINFO > For this package this can be ignored IMO > +1 for Ralf's comment Great! I wasn't sure if there was a precedent for this and it's good to know something can be packaged in this manner. Thank you for the answer. > *rpmlint - PASS > Umm??? Don't pass (see below) This is very strange indeed b/c the rpmlint passes for me with flying colors. [a@buildbox ~]$ rpmlint /sw/BUILDING/RPMS/i386/astyle-1.20.2-2.fc7.i386.rpm [a@buildbox ~]$ [a@buildbox ~]$ rpmlint --version rpmlint version 0.80 Copyright (C) 1999-2006 Frederic Lepied, Mandriva [a@buildbox ~]$ I'm using F7 as a build host. > *changelog - CHANGE > -- should remove the last changelog comment about a different version > Why? Why? Because it's stated in the guidelines that changelog comments should match the version of the pkg a person is packaging. If she wants to pkg a different version, she should have the comments for that version in its' spec file, and not in this one. > -- if another pkg exists with that version number then you should put > the comments in its' spec file > -- comment for initial version should match version of the pkg and > still exist so history of the pkg is maintained > Currently I don't understand what you want to say here The changelog contains comments for another version of this program. It is potentially confusing for a maintainer to have two versions of a pkg mentioned in the same spec file. I thought each version of pkg should have it's own rpm and spec file, am I incorrect? What I asked her to do was to replace the comment with one for the proper version like so: * Thu Sep 21 2006 Mary Ellen Foster <mefoster gmail com> 1.20.2-1 - Initial package > -- mock produces the following during the debug ... > cpio: astyle/<built-in>: No such file or directory > Can be ingored Great! Thanks for the info. > *debuginfo pkgs - PASS > Actually, don't pass (related with rpmlint - see below) Again, I had no problems with rpmlint. Am I doing something incorrectly? > *libraries - NEEDINFO > -- I think this package might need to be split apart into a libraries > pkg and a program pkg. > IMO this is not needed for this package. Great! Thanks for the info. Can you explain why a separate lib package isn't needed beyond what's in the documentation? > *parallel make - NEEDINFO > This is not needed for this package (g++ *.cpp meets the demand) So g++ will auto-optimize the job? Or do we not need it b/c they're individual source files already and will have their own job at compile time? > 4) LICENSING: > *need to correct the License: field - CHANGE Yes it needs to be set to License: LGPL. >* For rpmlint: >----------------------------------------------------------- >E: astyle-debuginfo script-without-shebang >/usr/src/debug/astyle/src/astyle_main.cpp >W: astyle-debuginfo spurious-executable-perm /usr/src/debug/astyle/src/astyle.h >E: astyle-debuginfo script-without-shebang >/usr/src/debug/astyle/src/ASEnhancer.cpp >E: astyle-debuginfo script-without-shebang >/usr/src/debug/astyle/src/ASResource.cpp >E: astyle-debuginfo script-without-shebang >/usr/src/debug/astyle/src/ASFormatter.cpp >E: astyle-debuginfo script-without-shebang >/usr/src/debug/astyle/src/ASBeautifier.cpp >------------------------------------------------------------- > - All of these are permission issues. Please fix these. I didn't get any of these errors. Please see my output. >* Macros > - /usr/bin/install <- use macros for /usr/bin. %{_bindir}/install should work nicely. Sorry I missed this... >* Documentation > - Please check if "install.html" is needed for %doc. This seems > to be needed for people who want to rebuild this package by > themselves and does not seem to be needed for rpm users. It contains information on what each component is used for, not just installation/build instructions so it seems relevant. But you're correct, it is more pertinent to those installing from source rather than rpm. I will try to be a little less verbose when doing reviews. Sorry for the length Mamoru and Ralf. (In reply to comment #15) > > General Review --> > >Well, If you want to do the review of this style, please make > >the summary of the review so that everyone (including submitter) can > >read your review easily. > > Hmmm... I thought I was being very thorough in my review. I followed every > aspect of the review process to the "T," but from now on will not post > everything at once. Let me put it this way: I find this style of reviews WAY too verbose to be useful. Submitters and reviewers are drowning in irrelevant (and partially bogus) comments. I for one only see 2 remaining issues: - GPL -> LGPL - Bogus permissions on source files. E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/astyle_main.cpp W: astyle-debuginfo spurious-executable-perm /usr/src/debug/astyle/src/astyle.h E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASEnhancer.cpp E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASResource.cpp E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASFormatter.cpp E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASBeautifier.cpp => Add chmod -x src/* to %prep All the rest is stylishness, not fixing anything. (In reply to comment #16) > (In reply to comment #15) > > > General Review --> > > >Well, If you want to do the review of this style, please make > > >the summary of the review so that everyone (including submitter) can > > >read your review easily. > > > > Hmmm... I thought I was being very thorough in my review. I followed every > > aspect of the review process to the "T," but from now on will not post > > everything at once. > Let me put it this way: I find this style of reviews WAY too verbose to be > useful. Submitters and reviewers are drowning in irrelevant (and partially > bogus) comments. I'm learning and appreciate the critique. In the future I will try to be more specific, and instead of listing out all of the checks I did, only list the ones that need changing; is that the style you are hinting at? :-) > I for one only see 2 remaining issues: > - GPL -> LGPL Check. > - Bogus permissions on source files. > E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/astyle_main.cpp > W: astyle-debuginfo spurious-executable-perm /usr/src/debug/astyle/src/astyle.h > E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASEnhancer.cpp > E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASResource.cpp > E: astyle-debuginfo script-without-shebang /usr/src/debug/astyle/src/ASFormatter.cpp > E: astyle-debuginfo script-without-shebang > /usr/src/debug/astyle/src/ASBeautifier.cpp Again, I'm not getting this with rpmlint. Can you let me know what version of rpmlint you have please? and what version of Fedora/RHEL you're using to build? Thanks. > => Add chmod -x src/* > to %prep Check. > All the rest is stylishness, not fixing anything. Ok. :-) Some of the points were raised for personal edification. :-) I will ask those in a different forum instead of through this review. The reason I asked them here in the first place was so everyone on this list could learn too. (In reply to comment #15) > > *rpmlint - PASS > > Umm??? Don't pass (see below) > > This is very strange indeed b/c the rpmlint passes > for me with flying colors. > > [a@buildbox ~]$ rpmlint /sw/BUILDING/RPMS/i386/astyle-1.20.2-2.fc7.i386.rpm > [a@buildbox ~]$ Well, when you rebuild srpm (by rpmbuild or mock), you should also have debuginfo rpm. For this package, there should be astyle-debuginfo rpm. Also, we have to check rpmlint error for src.rpm and the _installed_ rpm. > > *changelog - CHANGE > > -- should remove the last changelog comment > about a different version > > Why? > > Why? Because it's stated in the guidelines that changelog comments should > match the version of the pkg a person is packaging. This is for the *newest* (i.e. top) entry of the %changelog. > If she wants to pkg a > different version, she should have the comments for that version in its' spec > file, and not in this one. Well, please try "rpm -q --changelog coreutils", for example. BTW as this is NEEDSPONSOR blocker, will someone going to sponsor the reviewer, Ralf, Kevin (and me)? (In reply to comment #18) > (In reply to comment #15) > > > *rpmlint - PASS > > > Umm??? Don't pass (see below) > > > > This is very strange indeed b/c the rpmlint passes > > for me with flying colors. > > > > [a@buildbox ~]$ rpmlint /sw/BUILDING/RPMS/i386/astyle-1.20.2-2.fc7.i386.rpm > > [a@buildbox ~]$ > Well, when you rebuild srpm (by rpmbuild or mock), you should also > have debuginfo rpm. For this package, there should be > astyle-debuginfo rpm. Also, we have to check rpmlint error for > src.rpm and the _installed_ rpm. So I need to run rpmlint not only on the primary rpm, but also on the debug rpms and etc. Thank you for clarifying. :-) > > > *changelog - CHANGE > > > -- should remove the last changelog comment > > about a different version > > > Why? > > > > Why? Because it's stated in the guidelines that changelog comments should > > match the version of the pkg a person is packaging. > This is for the *newest* (i.e. top) entry of the %changelog. Good to know. I was under the wrong impression that versioning needed to be consistent with the pkg: I stand corrected. Thank you. > > If she wants to pkg a > > different version, she should have the comments for that version in its' spec > > file, and not in this one. > Well, please try "rpm -q --changelog coreutils", for example. Good to know. Thank you. > BTW as this is NEEDSPONSOR blocker, will someone going to sponsor > the reviewer, Ralf, Kevin (and me)? (In reply to comment #19) > > BTW as this is NEEDSPONSOR blocker, will someone going to sponsor > > the reviewer, Ralf, Kevin (and me)? Erm, ... the submitter needs to be sponsored, not the reviewer :) Mary, do you have any other packages of yours to be reviewed? (In reply to comment #20) > (In reply to comment #19) > > > BTW as this is NEEDSPONSOR blocker, will someone going to sponsor > > > the reviewer, Ralf, Kevin (and me)? > Erm, ... the submitter needs to be sponsored, not the reviewer :) Oops... (In reply to comment #20) > Mary, do you have any other packages of yours to be reviewed? Yes, and it's a bit bigger: I've also submitted Ice (object middleware). The review bug is here: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=234612 I'm slightly drowning in all of the recent comments, but I'll try to get a new astyle package out today or tomorrow. :) OK. I have a new spec, new debug,regular and source rpms up at: http://littlehat.homelinux.org:8000/FEDORA/RPMS/astyle/ I figured it was the least I could do for lobbing all the info onto this bug at once. Sorry Mary. :-/ I implemented all of Ralf's suggestions, but did not remove the html page Mamoru requested. If that's a definite I will, just let me know. rpmlint passed on the SRPM,i386-RPM and DEBUG rpm. Well, as no one is reviewing, assigning to me... * SourceURL - As this is sourceforge source, please follow: the section "Sourceforge.net" of http://fedoraproject.org/wiki/Packaging/SourceURL * Newest version - It seems 1.21 is released on Jun 21... So please update. >* SourceURL > - As this is sourceforge source, please follow: > the section "Sourceforge.net" of > http://fedoraproject.org/wiki/Packaging/SourceURL Source0: http://dl.sourceforge.net/%{name}/%{name}_%{version}_linux.tar.gz will change to Source0: http://downloads.sourceforge.net/%{name}/%{name}_%{version}_linux.tar.gz >* Newest version > - It seems 1.21 is released on Jun 21... So please update. Will grab and rebuild after changing spec file by Monday evening unless Mary does it before I do. (In reply to comment #25) > * SourceURL > - As this is sourceforge source, please follow: > the section "Sourceforge.net" of > http://fedoraproject.org/wiki/Packaging/SourceURL Fixed in the latest spec. > * Newest version > - It seems 1.21 is released on Jun 21... So please update. Fixed in the latest spec and build. Please see... http://littlehat.homelinux.org:8000/FEDORA/RPMS/astyle/current/ By the way, who will become the maintainer of this package? Adam, are you working together with Mary? I haven't heard from Mary. If she is unable to maintain the package I will. Can you please chime in Mary and let us know either way? Thanks. I've also not heard from Mary. I don't have a formal working relationship with Mary, I just wanted an updated astyle RPM package so I helped out a little. I'm happy to be a 3rd string maintainer. :) Mary, how do you think about maintainership? Also, I want you to update bug 234612 . I'd be happy for Adam to take over astyle. I'll reply on the Ice review shortly. I'm sorry, I've been travelling to conferences for the last two weeks; I'm back now and have more time to devote to this stuff again. Mamoru, how shall we all proceed? Well, I must clarify who will maintain this package. Monsen and Dutko, do you want to maintain this package? And will you also want to maintain this package, Mary? I'm willing to either co-maintain with Foster and Monsen or be primary, or be secondary; it's really up to what they'd prefer. Again, I'm OK with any scenario. OKay. Now I approve this package. -------------------------------------------------- This package (astyle) is APPROVED by me -------------------------------------------------- Dukto, please become the maintainer of this package. Monsen and Mary, if you also want to maintain this package, let us know on this bug ticket. Once removing NEEDSPONSOR blocker (In reply to comment #36) > OKay. Now I approve this package. > > -------------------------------------------------- > This package (astyle) is APPROVED by me > -------------------------------------------------- > > Dukto, please become the maintainer of this package. > Monsen and Mary, if you also want to maintain this package, > let us know on this bug ticket. > I will build for FC-6, F-7 and devel and also register for CVS inclusion this evening. Then once verified and built, push through Bodhi. > Once removing NEEDSPONSOR blocker New Package CVS Request ======================= Package Name: astyle Short Description: Source code formatter for C-like programming languages Owners: gnome Branches: FC6 F7 InitialCC: mtasaka.u-tokyo.ac.jp mefoster haircut cvs done. FC-6 PLAGUE BUILD DONE: http://buildsys.fedoraproject.org/build-status/job.psp?uid=34797 FC-7 KOJI BUILD FAILED: BuildError: package astyle not in list for tag dist-fc7-updates-candidate I think it needs to be promoted to fc7-updates-candidate? F-8 KOJI BUILD FAILED: [a@buildbox devel]$ make build astyle.spec not tagged with tag astyle-1_21-5_fc8 make: *** [build-check] Error 1 I tried to remove from cvs, then readd and even when I do make tag build, it says the tag already exists... I don't see how to remove the tag after it's been created. I'm sleepy so please forgive me if I'm overlooking something simple. Any ideas? You may have to wait for about one or two hours... Note: I live in Japan and now it is 11:40 AM. Perhaps now you can rebuild astyle on koji. (In reply to comment #42) > Perhaps now you can rebuild astyle on koji. FC-7 KOJI BUILD SUCCEEDED: http://koji.fedoraproject.org/koji/buildinfo?buildID=10510 F-8 KOJI BUILD FAILED: [a@buildbox devel]$ make build astyle.spec not tagged with tag astyle-1_21-5_fc8 make: *** [build-check] Error 1 Not sure what I need to do with the F-8 build failure. I'm thinking I might need to delete the directory from CVS and then recreate it? I've pushed astyle in bodhi: Release: Fedora 7 Status: pending Type: enhancement Bugs: 207896 - Review Request: astyle - Source code formatter Requested: push Pushed: False Mail Sent: False ... Submitted: 2007-07-06 04:58:40 Build Logs: http://koji.fedoraproject.org/packages/astyle/1.21/5.fc7/data/logs The last thing to fix is the devel build issue. Any ideas? Hi Adam, Did you import the src or tarball in the CVS branch devel? http://cvs.fedora.redhat.com/viewcvs/rpms/astyle/devel/?root=extras&only_with_tag=astyle-1_21-5_fc8 shows that astyle.spec is not tagged. The easiest way to fix this is to bump release number and retag. Will attempt to fix when I get home this evening. Thank you for the suggestions. astyle-1.21-5.fc7 has been pushed to the Fedora 7 testing repository. If problems still persist, please make note of it in this bug report. Build for F-8 branch completed successfully: http://koji.fedoraproject.org/koji/buildinfo?buildID=10605 All branches built, and F-7 in testing repository. DONE. astyle-1.21-5.fc7 has been pushed to the Fedora 7 stable repository. If problems still persist, please make note of it in this bug report. |