Closed Bug 556394 Opened 14 years ago Closed 14 years ago

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

Categories

(MailNews Core :: Build Config, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Callek, Assigned: Callek)

Details

Attachments

(2 files, 2 obsolete files)

Attached patch v1.0 (obsolete) — Splinter Review
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.
Attachment #436315 - Flags: review?(gozer)
Attachment #436315 - Flags: feedback?(kairo)
Assignee: nobody → bugspam.Callek
whops, missed one required check
Attachment #436315 - Attachment is obsolete: true
Attachment #436319 - Flags: review?(gozer)
Attachment #436319 - Flags: feedback?(kairo)
Attachment #436315 - Flags: review?(gozer)
Attachment #436315 - Flags: feedback?(kairo)
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.
(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)
(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?
Attachment #436319 - Flags: review?(gozer)
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.
Attachment #436319 - Flags: feedback?(kairo)
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.
Attachment #436319 - Attachment is obsolete: true
Attachment #450994 - Flags: review?(gozer)
Attachment #450994 - Flags: feedback?(kairo)
Attachment #450994 - Flags: review?(gozer) → review+
Attached patch Part 2 (v1)Splinter Review
This is the patch that fixes this bug; applied on top of Part 1
Attachment #451460 - Flags: review?(gozer)
Attachment #451460 - Flags: feedback?(kairo)
Attachment #450994 - Flags: feedback?(kairo) → feedback+
Comment on attachment 451460 [details] [diff] [review]
Part 2 (v1)

I didn't test but it looks like it's doing the right thing.
Attachment #451460 - Flags: feedback?(kairo) → feedback+
Comment on attachment 450994 [details] [diff] [review]
v2 (Part 1) [checked-in]

http://hg.mozilla.org/comm-central/rev/106acab5b333
Attachment #450994 - Attachment description: v2 (Part 1) → v2 (Part 1) [checked-in]
Attachment #451460 - Flags: review?(gozer) → review+
Pushed as: http://hg.mozilla.org/comm-central/rev/b999fdbf2107
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Mark/KaiRo: do we want a backport of this feature for 1.9.1/1.9.2? or should we just avoid it?
(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...
(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).
You need to log in before you can comment on or make changes to this bug.