Closed
Bug 1041789
Opened 10 years ago
Closed 3 years ago
Efficiency + clarity improvements in pastebin script
Categories
(Firefox Build System :: Mach Core, enhancement)
Firefox Build System
Mach Core
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: ianconnolly, Unassigned)
Details
Attachments
(1 file, 1 obsolete file)
4.78 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•10 years ago
|
Assignee: nobody → ffledgling
Status: NEW → ASSIGNED
Updated•10 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Comment 1•10 years ago
|
||
Adding fix for the aforementioned problems.
Also did some other cleanup inside the code while I was at it.
Updated•10 years ago
|
Attachment #8467640 -
Flags: feedback?(gps)
Comment 2•10 years ago
|
||
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+
Comment 3•10 years ago
|
||
Sorry about the oversight, fixed.
Attachment #8467640 -
Attachment is obsolete: true
Attachment #8468544 -
Flags: review?(gps)
Comment 4•10 years ago
|
||
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+
Updated•7 years ago
|
Product: Core → Firefox Build System
Comment 5•3 years ago
|
||
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)
Updated•3 years ago
|
Flags: needinfo?(mhentges)
Comment 6•3 years ago
|
||
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.
Description
•