Re: treelang patch part 1 of 6

[Geoff Keating <geoffk at geoffk dot org>]

Tim Josling <tej@melbpc.org.au> writes:

> This is about to be applied to the default branch (gcc3.2 experimental).
> 
> Tim Josling
> 
> diff -c -r -p -N -X treelang.diff.excl
> cvs/gcc/gcc/testsuite/lib/treelang.exp
> cvscopy/gcc/gcc/testsuite/lib/treelang.exp
> *** cvs/gcc/gcc/testsuite/lib/treelang.exp      Thu Jan  1 10:00:00 1970
> 
> --- cvscopy/gcc/gcc/testsuite/lib/treelang.exp  Mon Apr 29 20:30:08 2002

Unfortunately, your patch was so mangled I couldn't review all of it.
However, here are some comments:

* It seems to have many lines that are > 80 characters, and your
  posting program word-wrapped it---this is two problems in one, since
  generally source lines should be less than 80 characters, as well as
  the word-wrapping.

* It got split into many pieces, which makes it difficult to search
  the whole patch for things.  It may be better for you to put the
  patch on a web site somewhere, since it seems too large to post
  uncompressed.

* Could you put the ChangeLog entries at the top of the patch?

* Can you re-check your copyright dates?  I disbelieve that it took 14
  years to write the comment

> + # Having this file here magically tells dejagnu that the treelang
> + # directory is worthy of testing

  in the file treelang.exp.

  Oh, and the comment is missing a period ('.') at the end of the
  sentence.

* Some files don't have the full GPL copyright notice.  It is OK to
  have no notice, if the file is small, but it is not OK to have a
  notice that just claims copyright without granting any permission to
  copy.

* Also, some of the copyright notices are not in the right form; they
  are missing comments, they don't have 4-digit years, they use year
  ranges (like "85-93"), there are comments missing, and so on.
  Please check that they are all exactly right.  Please exactly follow
  the format in COPYING, putting the notice at the very top of the
  source file; this helps when the notices are automatically updated.

* The following ChangeLog entry is missing a blank line, a colon, and
  a period:

+ 2002-04-13  Tim Josling  <tej@melbpc.org.au>
+       * treetree.c (tree_code_create_function_initial)
+       Remove duplicate call to layout_decl

  There are many other ChangeLog entries that seem to be badly
  formatted.

* There is documentation, but shouldn't it be in gcc/doc rather than
  in treelang/doc?

Please correct these and re-submit the patch.

-- 
- Geoffrey Keating <geoffk@geoffk.org> <geoffk@redhat.com>