[PATCH 1 of 2 V2] push: remove undocumented exit code 2 for bookmark exchange failure

Pierre-Yves David pierre-yves.david at logilab.fr
Tue Dec 11 11:27:28 CST 2012


On Mon, Dec 10, 2012 at 12:02:55PM -0600, Kevin Bullock wrote:
> On Dec 10, 2012, at 3:50 AM, Pierre-Yves David wrote:
> 
> > On Fri, Dec 07, 2012 at 01:43:15PM -0600, Kevin Bullock wrote:
> >> # HG changeset patch
> >> # User Kevin Bullock <kbullock at ringworld.org>
> >> # Date 1354908879 21600
> >> # Node ID 106b515749cf338617d24c2fae3850987bf46734
> >> # Parent  f3991bcf4f0ff43b43a1b1d0210925a629ef3b9c
> >> push: remove undocumented exit code 2 for bookmark exchange failure
> >> 
> >> This exit code was originally added in e1a145eebb6a, but was never
> >> documented. It interferes with moving the remaining bookmark exchange
> >> code into localrepo.push(), because that method returns an integer
> >> indicating the number of heads added.
> > 
> > Dropping useful information does not seems a win, but according your next patche
> > we did not get information for already shared bookmark.
> > 
> > Maybe it's time to have a richer return from push?
> 
> That would be the alternative, since dropping this exit code doesn't seem to be desired. I'm hesitant to add a multi-valued return here though. I'm open to suggestions on how to do this minimally-disruptively.

push now synchronise a lot of information:

- changeset
- bookmark
- phase (both way)
- obsolescence marker

We will want to have information about all that at some point. You can delay
that but that is probably unavoidable.

If you want to take the least distrubtive path, you can return a custom object
that behave as an int for legacy user.

> >> diff --git a/mercurial/commands.py b/mercurial/commands.py
> >> --- a/mercurial/commands.py
> >> +++ b/mercurial/commands.py
> >> @@ -4784,13 +4784,11 @@ def push(ui, repo, dest=None, **opts):
> >>             else:
> >>                 ui.warn(_('bookmark %s does not exist on the local '
> >>                           'or remote repository!\n') % b)
> >> -                return 2
> >> +                break
> > 
> > I think we want a continue here, isn't it?
> > Trying to honor all -B options seems more sensible. This probaby deserve
> > another patch anyway.
> 
> Likely so. I considered it but decided to stick to moving the existing code without changing any more behavior than the exit code.

That should be in another patch, but if you do not do it in this series, it
will remains as is until an angry user fill a bug someday.


-- 
Pierre-Yves David

http://www.logilab.fr/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20121211/13dc0fa7/attachment.pgp>


More information about the Mercurial-devel mailing list