Hi Matthijs,
On Thu, Aug 19, 2010 at 02:26:01PM +0200, Matthijs Kooijman wrote:
> Hi Guido,
> 
> I've rebased the patch against current git master, and also coded up a
> "read the changelog from the repository"-feature. The latter was a bit
> complicated, so I split it up in a few patches. There's also an
> unrelated variable name fix, that I came across while reading the code.
> 
> Is this what you had in mind?
Yes. This looks really great in general, thanks alot! Two minor
nitpicks:

 * It'd be nicer if things like packagename_re and upstreamversion_re
   would end up in gbp/deb.py (together with functions like
   is_valid_{packagename,upstreamversion}

 * Always reading the whole changelog via read seems a bit like
  overkill. Wouldn't it be nicer if parse_changelog could either take a 
  buffer or a filename? parse_changelog_file and parse_changelog_repo
  would then invoke the above function with the right arguments like:

      parse_changelog(filename='debian/changelog')
      parse_changelog(content='...')

I wanted to apply [1/6] since we can clean that up with follow up
patches but id doesn't seem to apply against the experimental branch.
Cheers,
 -- Guido




-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to