Closed Bug 893830 Opened 7 years ago Closed 7 years ago

Implement mach uuid

Categories

(Firefox Build System :: Mach Core, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla25

People

(Reporter: graememcc, Assigned: graememcc)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Attached patch v1 (obsolete) — Splinter Review
Sometimes, asking firebot or remembering the name of your native OS command is just too much work.
Attachment #775675 - Flags: review?(gps)
Comment on attachment 775675 [details] [diff] [review]
v1

Review of attachment 775675 [details] [diff] [review]:
-----------------------------------------------------------------

::: tools/mach_commands.py
@@ -10,4 @@
>      Command,
>  )
>  
> -

Does pep8 say anything about the number of empty lines here?

@@ +68,5 @@
> +    def uuid(self, format):
> +        import uuid
> +        u = uuid.uuid4()
> +        if format == 'idl':
> +          print(u)

4-space

@@ +73,5 @@
> +        else:
> +            u = u.hex
> +            print('{ 0x%s, 0x%s, 0x%s, \\' % (u[0:8], u[8:12], u[12:16]))
> +            pairs = tuple(map(lambda n: u[n:n+2], range(16, 32, 2)))
> +            print((' { ' + '0x%s, ' * 7 + '0x%s } }') % pairs)

Double space before the { to match http://mozilla.pettay.fi/cgi-bin/mozuuid.pl
Attached patch v2Splinter Review
> Does pep8 say anything about the number of empty lines here?
Bah, originally misguidedly placed my import up top; must have inadvertently deleted that line when moving the import.
Attachment #775675 - Attachment is obsolete: true
Attachment #775675 - Flags: review?(gps)
Attachment #775703 - Flags: review?(gps)
Comment on attachment 775703 [details] [diff] [review]
v2

Review of attachment 775703 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!
Attachment #775703 - Flags: review?(gps) → review+
graememcc++

Will be worth posting this on the newsgroups after it lands too :-)
Assignee: nobody → graememcc_firefox
https://hg.mozilla.org/mozilla-central/rev/a639cb76d4d9
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Following my post on planet, both mstange and njn have suggested that outputting both forms would be more useful. This would match Smaug's online tool, and the format parameter will still allow control of the output format.

I added c++ as a more natural format specification in addition to cpp.
Attachment #777733 - Flags: review?(gps)
Comment on attachment 777733 [details] [diff] [review]
Followup: output both forms by default

Review of attachment 777733 [details] [diff] [review]:
-----------------------------------------------------------------

WFM (with nits addressed).

::: tools/mach_commands.py
@@ +68,2 @@
>                       help='Output format for the generated uuid.')
>      def uuid(self, format):

You should say "format=None" because that argument will be passed by name, not position. You are getting lucky here because there is only 1 non-self argument and there's no potential for confusion.

@@ +73,2 @@
>              print(u)
> +            if format == None:

if format is None:

or

if not format:
Attachment #777733 - Flags: review?(gps) → review+
You know those times when you intend to land a patch, forget to push it, and it lies gathering dust in Bugzilla for a few months? Yep. Mea culpa.

Pushed with nits addressed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/91690a35f26d
Product: Core → Firefox Build System
Blocks: 1451993
You need to log in before you can comment on or make changes to this bug.