Efficiency + clarity improvements in pastebin script

ASSIGNED
Assigned to

Status

--
enhancement
ASSIGNED
4 years ago
7 months ago

People

(Reporter: ianconnolly, Assigned: ffledgling)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
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)

Updated

4 years ago
Assignee: nobody → ffledgling
Status: NEW → ASSIGNED
(Assignee)

Updated

4 years ago
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 1

4 years ago
Created attachment 8467640 [details] [diff] [review]
Bug1041789.patch

Adding fix for the aforementioned problems.
Also did some other cleanup inside the code while I was at it.
(Assignee)

Updated

4 years ago
Attachment #8467640 - Flags: feedback?(gps)

Comment 2

4 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+
(Assignee)

Comment 3

4 years ago
Created attachment 8468544 [details] [diff] [review]
Bug1041789.patch

Sorry about the oversight, fixed.
Attachment #8467640 - Attachment is obsolete: true
Attachment #8468544 - Flags: review?(gps)

Comment 4

4 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 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.