556394 - Allow a single option to control tag/revision to pull in client.py

Status: RESOLVED FIXED

(MailNews Core :: Build Config, defect)

Description

[Justin Wood (:Callek)]

Attachment: v1.0

This patch is currently untested, but I think it will work.

feedback-only from KaiRo, as he is not great with python. (Please also help test)
review from gozer.

Comment 1

[Justin Wood (:Callek)]

Attachment: v1.1 -- don't overwrite an explicit rev

whops, missed one required check

Comment 2

[Robert Kaiser]

Comment on attachment 436319 [details] [diff] [review]
v1.1 -- don't overwrite an explicit rev

1) I like the older layout of the default vars better
2) I don't think we should apply this to CVS, as a) we don't tag LDAP for out releases and b) CVS doesn't support a "default" revision.
3) why do we need to hand a default revision over to do_hg_pull() at all? Can't we just check if rev is set and else fall back to the globally set variable?
4) The --rev description should say "Named tag" or something instead of "Revision", as the latter term implies you can hand over specific revision IDs there, which is wrong.

Comment 3

[Justin Wood (:Callek)]

(In reply to comment #2)
> (From update of attachment 436319 [details] [diff] [review])
> 1) I like the older layout of the default vars better

Ok, I do prefer this setup; but I won't argue the point

> 2) I don't think we should apply this to CVS, as a) we don't tag LDAP for out
> releases and b) CVS doesn't support a "default" revision.

After I did it I thought on this as well. So I'll revert

> 3) why do we need to hand a default revision over to do_hg_pull() at all? Can't
> we just check if rev is set and else fall back to the globally set variable?

if we don't we will never get any passed rev/tag from command line. If you don't think a command-line option is worth it, this can probably be simplified.

> 4) The --rev description should say "Named tag" or something instead of
> "Revision", as the latter term implies you can hand over specific revision IDs
> there, which is wrong.

Thats no problem; but dependant on 3)

Comment 4

[Robert Kaiser]

(In reply to comment #3)
> if we don't we will never get any passed rev/tag from command line. If you
> don't think a command-line option is worth it, this can probably be simplified.

I think a command-line option is why I requested this in the first place... How else could we make something like the checkout instructions on http://releases.mozilla.org/pub/mozilla.org/seamonkey/releases/2.0.4/README nicer?

Comment 5

[Robert Kaiser]

Comment on attachment 436319 [details] [diff] [review]
v1.1 -- don't overwrite an explicit rev

I provided my feedback in comments, but can't decide if that means + or - so canceling the request.

Comment 6

[Justin Wood (:Callek)]

Attachment: v2 (Part 1) [checked-in]

Ok, this is part 1 of two parts I am splitting this bug into.

This part creates a python dictionary for DEFAULTS and stuffs our (main) defaults in there, and adds the file-level ability to set a DEFAULT REV.

The commandline option to do this will be in the next patch, awaiting review first.

Comment 7

[Justin Wood (:Callek)]

Attachment: Part 2 (v1)

This is the patch that fixes this bug; applied on top of Part 1

Comment 8

[Robert Kaiser]

Comment on attachment 451460 [details] [diff] [review]
Part 2 (v1)

I didn't test but it looks like it's doing the right thing.

Comment 9

[Justin Wood (:Callek)]

Comment on attachment 450994 [details] [diff] [review]
v2 (Part 1) [checked-in]

http://hg.mozilla.org/comm-central/rev/106acab5b333

Comment 10

[Justin Wood (:Callek)]

Pushed as: http://hg.mozilla.org/comm-central/rev/b999fdbf2107

Comment 11

[Justin Wood (:Callek)]

Mark/KaiRo: do we want a backport of this feature for 1.9.1/1.9.2? or should we just avoid it?

Comment 12

[Robert Kaiser]

(In reply to comment #11)
> Mark/KaiRo: do we want a backport of this feature for 1.9.1/1.9.2? or should we
> just avoid it?

I personally don't care about 1.9.2 much, but I'm ambiguous to backporting as I really would love to have this for making release pull instructions easier for those who build from release tags themselves, but OTOH this is a rather large-ish patch in terms of client.py...

Comment 13

[Mark Banner (:standard8)]

(In reply to comment #12)
> (In reply to comment #11)
> > Mark/KaiRo: do we want a backport of this feature for 1.9.1/1.9.2? or should we
> > just avoid it?
> 
> I personally don't care about 1.9.2 much, but I'm ambiguous to backporting as I
> really would love to have this for making release pull instructions easier for
> those who build from release tags themselves, but OTOH this is a rather
> large-ish patch in terms of client.py...

I think we can take it after we've done a release or two on it - i.e. so as I think SM has already released trunk with it, then after TB does the first alpha off of trunk (probably in a couple of weeks).