[MERGE] bzr rm improvements

[Robert Collins]

On Wed, 2008-08-13 at 18:14 +1000, Andrew Bennetts wrote:
> Robert Collins wrote:

> > === modified file 'bzrlib/builtins.py'
> [...]
> >  class cmd_remove(Command):
> >      """Remove files or directories.
> >  
> > -    This makes bzr stop tracking changes to the specified files and
> > -    delete them if they can easily be recovered using revert.
> > -
> > -    You can specify one or more files, and/or --new.  If you specify --new,
> > -    only 'added' files will be removed.  If you specify both, then new files
> > -    in the specified directories will be removed.  If the directories are
> > -    also new, they will also be removed.
> > +    This makes bzr stop tracking changes to the specified files and delete them
> 
> s/delete/deletes

As discussed on IRC, thats actually incorrect. (Subject-verb agreement). 

However, I've taken your proprosed rephrasing into two sentences.

> === modified file 'bzrlib/tests/blackbox/test_remove.py'
> [...]
> > -    def _make_add_and_assert_tree(self, files):
> > +    def _make_add_and_assert_tree(self, paths):


> If this method is no longer making any assertions, then it shouldn't have
> “and_assert” in the name.  (I do agree with removing the asserts from this
> helper method, I just want the name to be updated.)

I was trying to avoid noise, but I'll change the name happily :).

> [...]
> >      def test_remove_no_files_specified(self):
> [...]
> > -        self.run_bzr_error(["bzr: ERROR: Specify one or more files to remove, "
> > -            "or use --new."], 'remove')
> > -
> > -        self.run_bzr_error(["bzr: ERROR: No matching files."], 'remove --new')
> > -
> > -        self.run_bzr_error(["bzr: ERROR: No matching files."],
> > -            'remove --new .')
> 
> Hmm, it looks like you're losing some test coverage here.  There's nowhere else
> in this file that exercises the “No matching files” error path, or the use of
> “remove --new” with no other arguments (although this latter condition may be
> less interesting?).
> 
> So, please add a test that exercises the “No matching files” case of “bzr remove
> --new”.

This is a good catch - that test did not do just what it said.

-Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080815/d16bbbdf/attachment.pgp