[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: gem2deb: setup.rb branch ready for merging with master



Vincent Fourmond escreveu isso aí:
>   Hello Antonio,
> 
> On Wed, Jun 1, 2011 at 3:13 AM, Antonio Terceiro
> <terceiro@softwarelivre.org> wrote:
> >>   Attached is .tar.gz archive of the current contents of the setuprb
> >> branch that adds support for setup.rb installations to gem2deb. It now
> >> contains tests (that even pass!), so I think it is complete enough for
> >> merging into master. Unless someone has objections to that, I plan to
> >> do perform the merge some time later this week.
> >
> > about the last patch (the one that adds tests):
> >
> > - please add your new tests to test/unit/dh_ruby_setuprb_test.rb
> >  instead, to keep them separated from the tests for the original class.
> >
> > - it would be nice if your tests tested some functionality of the
> >  setup.rb build (e.g. the hooks)
> >
> > - it would be nice to s/simpleextension/simplesetuprb/ in the sample
> >  package you are adding (both in contents and filenames).
> 
>   Attached is another output of git-format-patch that does all of
> that. Unless you spot something wrong in them, I'd be glad to merge
> them myself to master this time ;-)...

Very nice, thanks. Don't want to nitpick, but I didn't like the
duplication of the test helper methods (assert_installed, build and
friends). Any test helpers that are needed by more than one test class
should be moved up to their superclass.

BTW, if you use a proper branch to publish your changes in the
repository itself (except the master branch), or even if you `git
send-email` all the patches to the list, it would be easier for you and
for me. :)

-- 
Antonio Terceiro <terceiro@softwarelivre.org>
http://softwarelivre.org/terceiro


Attachment: signature.asc
Description: Digital signature


Reply to: