Closed
Bug 1344718
Opened 8 years ago
Closed 8 years ago
Enable flake8 rule E302: "expected 2 blank lines, found 1"
Categories
(Toolkit :: Telemetry, enhancement, P3)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla55
People
(Reporter: Dexter, Assigned: djmdeveloper060796, Mentored)
References
Details
(Whiteboard: [measurement:client][lang=python])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Dexter
:
review+
|
Details | Diff | Splinter Review |
In bug 1332651 we landed flake8 initial support to lint Python files in the Telemetry directory, by disabling all the detected problems.
This bug is about enabling the E302 rule: "expected 2 blank lines, found 1":
1) Remove the E302 rule from the .flake8 file in https://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry
2) Run "./mach lint -l flake8 toolkit/components/telemetry".
3) Fix the reported problems
Reporter | ||
Updated•8 years ago
|
Points: --- → 1
Priority: -- → P3
Whiteboard: [measurement:client][lang=python]
Assignee | ||
Comment 1•8 years ago
|
||
@Alessio I would like to work on this, would please assign me to this ?
Flags: needinfo?(alessio.placitelli)
Reporter | ||
Comment 2•8 years ago
|
||
(In reply to Deepjyoti Mondal from comment #1)
> @Alessio I would like to work on this, would please assign me to this ?
Before this bug becomes actionable, we need the dependency (Enabling flake8) to land! :)
Once it does, if no problem arises and you have some spare time, I'll gladly do!
Flags: needinfo?(alessio.placitelli)
Assignee | ||
Comment 3•8 years ago
|
||
okay :)
Assignee | ||
Comment 4•8 years ago
|
||
Enabled flake8 rule E302
Attachment #8844346 -
Flags: review?(alessio.placitelli)
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → djmdeveloper060796
Reporter | ||
Comment 5•8 years ago
|
||
Comment on attachment 8844346 [details] [diff] [review]
bug1344718.patch
Review of attachment 8844346 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks for the patch, this looks good overall. However, if you apply it on the most recent version of mozilla-central, you will get:
> shared_telemetry_utils.py
> 24:1 error expected 2 blank lines, found 1 E302 (flake8)
> 27:1 error expected 2 blank lines, found 1 E302 (flake8)
> ? 2 problems (2 errors, 0 warnings)
Please fix both histogram_tools.py and shared_telemetry_utils.py.
::: toolkit/components/telemetry/histogram_tools.py
@@ +431,4 @@
> # just Histograms.json. For each file's basename, we have a specific
> # routine to parse that file, and return a dictionary mapping histogram
> # names to histogram parameters.
> +
Instead of adding the newlines after the comment block, could you add them before?
Attachment #8844346 -
Flags: review?(alessio.placitelli) → feedback+
Assignee | ||
Comment 6•8 years ago
|
||
I have updated my mozilla-central repo and have made the changes. Its not showing any error.
Attachment #8844346 -
Attachment is obsolete: true
Attachment #8844985 -
Flags: review?(alessio.placitelli)
Reporter | ||
Comment 7•8 years ago
|
||
Comment on attachment 8844985 [details] [diff] [review]
bug1344718.patch
Review of attachment 8844985 [details] [diff] [review]:
-----------------------------------------------------------------
This looks good now, thanks!
For the next time, could you please append the name of the reviewer at the end of the patch? For example, the current commit message would become:
"Bug 1344718 - Enabled flake8 rule E302: "expected 2 blank lines, found 1". r?dexter"
See https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#Commit_Message_Conventions
Attachment #8844985 -
Flags: review?(alessio.placitelli) → review+
Reporter | ||
Comment 8•8 years ago
|
||
Assignee | ||
Comment 9•8 years ago
|
||
Sure, I wont miss it next time :)
Reporter | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 10•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/50a0ca4800c1
Enable flake8 rule E302: "expected 2 blank lines, found 1" for Telemetry code. r=Dexter
Keywords: checkin-needed
Comment 11•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•