Closed Bug 1041789 Opened 10 years ago Closed 3 years ago

Efficiency + clarity improvements in pastebin script

Categories

(Firefox Build System :: Mach Core, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ianconnolly, Unassigned)

Details

Attachments

(1 file, 1 obsolete file)

You unnecessarily iterate FILE_TYPES when the user passes args.lang. Also, this is better as a top-level dict (with what you call 'value' as the key) rather than a list of dicts to speed up lookup in the vast majority of cases.
Assignee: nobody → ffledgling
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Attached patch Bug1041789.patch (obsolete) — Splinter Review
Adding fix for the aforementioned problems. Also did some other cleanup inside the code while I was at it.
Attachment #8467640 - Flags: feedback?(gps)
Comment on attachment 8467640 [details] [diff] [review] Bug1041789.patch Review of attachment 8467640 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/mach_commands.py @@ +262,5 @@ > + if not lang: > + extension = os.path.splitext(filename)[1][1:] > + if extension: > + lang = FILE_TYPES[extension]['value'] > + name = FILE_TYPES[extension]['name'] These will raise a KeyError if |extension not in FILE_TYPES|.
Attachment #8467640 - Flags: feedback?(gps) → feedback+
Attached patch Bug1041789.patchSplinter Review
Sorry about the oversight, fixed.
Attachment #8467640 - Attachment is obsolete: true
Attachment #8468544 - Flags: review?(gps)
Comment on attachment 8468544 [details] [diff] [review] Bug1041789.patch Review of attachment 8468544 [details] [diff] [review]: ----------------------------------------------------------------- r+ conditional on incorporation of style nit. I trust you to land it properly. ::: tools/mach_commands.py @@ +268,5 @@ > + except KeyError: > + print('Could not identify filetype') > + else: > + name = FILE_TYPES[extension]['name'] > + print('Identified file as %s' % name) This would be much easier to read if it were written as: try: lang = FILE_TYPES[extension]['value'] name = FILE_TYPES[extension]['name'] print('Identified file as %s' % name) except KeyError: print('Could not identify filetype')
Attachment #8468544 - Flags: review?(gps) → review+
Product: Core → Firefox Build System

The bug assignee didn't login in Bugzilla in the last 7 months.
:mhentges, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: ffledgling → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(mhentges)
Flags: needinfo?(mhentges)

There's no longer a FILE_TYPES object in tools/mach_commands.py

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: