Corinna Vinschen wrote:
> > Here's the patch that my colleague did:
> > http://www.tcm.phy.cam.ac.uk/~mr349/cygwin.patch
> > It is definitely not the best code, but it works and meets our needs and
> > maybe will be helpful to the cygwin project.
>
> Wrong mailing list. setup.exe is handled entirely on cygwin-apps.
> See http://cygwin.com/lists.html
I have no objection to the spirit of this patch, however:
- The formatting does not follow the GCS, for example:
+bool
+packagemeta::isManuallyWanted() const
+{
+ string packages_option = PackageOption;
+ string tname;
+ /* Split the packages listed in the option up */
+ string::size_type loc = packages_option.find( ",", 0 );
+ bool breturn=false;
+ while ( loc != string::npos ) {
+ tname=packages_option.substr(0,loc);
+ packages_option=packages_option.substr(loc+1);
+ breturn = breturn || (name.compare(tname)==0);
+ loc = packages_option.find( ",", 0 );
+ }
+ /* At this point, no "," exists */
+ breturn=breturn || (name.compare(packages_option)==0);
+ return breturn;
+}
+
The whitespace and brace style is inconsistent. There should always be
space on either side of "=" and before "("; the opening and closing
braces go on their own lines; there should be a period and two spaces at
the end of a comment; etc.
- There was no ChangeLog included.
- Please not include changes to generated files such as setup_version.c.
- Adding a new command line option causes the FAQ to be out of date
(http://cygwin.com/faq/faq.setup.html#faq.setup.cli) so please submit a
separate patch to update the FAQ as well.
Brian
