| Message ID | 1282934717.625.33.camel@glinka |
|---|---|
| State | New |
| Headers | show |
I believe the patch is OK, but: - do not include mailing list links in the ChangeLog - Capitalize the "Test" Thank you for catching this and Ian for the explanation. 2010/8/27 Basile Starynkevitch <basile@starynkevitch.net>: > Hello All, > > A half line patch, discussed on > http://gcc.gnu.org/ml/gcc/2010-08/msg00396.html > http://gcc.gnu.org/ml/gcc/2010-08/msg00386.html > > Attached files: > the diff > the gcc/ChangeLog entry. > > Ok for trunk? > > Cheers. [Basile Starynkevitch & Jeremie Salvucci] > > -- > Basile STARYNKEVITCH http://starynkevitch.net/Basile/ > email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359 > 8, rue de la Faiencerie, 92340 Bourg La Reine, France > *** opinions {are only mine, sont seulement les miennes} *** >
Basile Starynkevitch <basile@starynkevitch.net> writes: > Hello All, > > A half line patch, discussed on > http://gcc.gnu.org/ml/gcc/2010-08/msg00396.html > http://gcc.gnu.org/ml/gcc/2010-08/msg00386.html > > Attached files: > the diff > the gcc/ChangeLog entry. > > Ok for trunk? > > Cheers. [Basile Starynkevitch & Jeremie Salvucci] 2010-08-27 Basile Starynkevitch <basile@starynkevitch.net> Jeremie Salvucci <jeremie.salvucci@free.fr> http://gcc.gnu.org/ml/gcc/2010-08/msg00396.html * gengtype.c (output_type_enum): test the right union member. Please omit the HTTP link, and capitalize "test". This is OK with those ChangeLog changes. Thanks. Ian
> > Please omit the HTTP link, and capitalize "test". > > This is OK with those ChangeLog changes. I did comply, and committed 163596. However, I am curious. Why is it wrong to make a link to something under gcc.gnu.org. For bugs with a PR, there is a number which is logically such a link. Cheers.
Basile Starynkevitch <basile@starynkevitch.net> writes: > However, I am curious. Why is it wrong to make a link to something > under gcc.gnu.org. For bugs with a PR, there is a number which is > logically such a link. The ChangeLog says what has changed, it does not say why it has changed. If it is necessary to say why something has changed, the place for that is in a comment in the code. You are right that the bug numbers in the ChangeLog entries are an exception. They are there to cause the patch to be automatically added to the bug report. You may, if you like, present an argument for why we should have the ChangeLog file link to the mailing list. But then we should do it for pretty much every change, not just yours. Unless and until we make that general change, you should follow the existing conventions. Of course, these days, with use of source code control and fast search of code repositories, it's not clear that we need a the current format of the ChangeLog file at all. Ian
Index: gcc/gengtype.c =================================================================== --- gcc/gengtype.c (revision 163593) +++ gcc/gengtype.c (working copy) @@ -2531,7 +2531,7 @@ write_types_process_field (type_p f, const struct static void output_type_enum (outf_p of, type_p s) { - if (s->kind == TYPE_PARAM_STRUCT && s->u.s.line.file != NULL) + if (s->kind == TYPE_PARAM_STRUCT && s->u.param_struct.line.file != NULL) { oprintf (of, ", gt_e_"); output_mangled_typename (of, s);