Fix #7868: Added lint check for a newline above args in python doc string #7929

Hudda · 2019-11-06T03:46:55Z

Explanation

Fixes Add lint check to ensure that there is a newline above args in python doc string #7868
Added lint check to ensure that there is a new line above args in python doc string.

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
The linter/Karma presubmit checks have passed.
- These should run automatically, but if not, you can manually trigger them locally using python -m scripts.pre_commit_linter and python -m scripts.run_frontend_tests.
The PR is made from a branch that's not called "develop".
The PR has an appropriate "PROJECT: ..." label (Please add this label for the first-pass review of the PR).
The PR has an appropriate "CHANGELOG: ..." label (If you are unsure of which label to add, ask the reviewers for guidance).
The PR follows the style guide.
The PR addresses the points mentioned in the codeowner checks for the files/folders changed. (See the codeowner's wiki page.)
The PR is assigned to an appropriate reviewer.
- If you're a new contributor, please ask on Gitter for someone to assign a reviewer and don't tick this checkbox.
- If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue. Do not only request the review but also add the reviewer as an assignee.

oppiabot · 2019-11-06T03:46:57Z

Hi, @Hudda. This pull request does not have a "CHANGELOG: ..." label as mentioned in the PR checkbox list. Please add this label. PRs without this label will not be merged. If you are unsure of which label to add, please ask the reviewers for guidance. Thanks!

oppiabot · 2019-11-06T03:59:40Z

Assigning @kevinlee12 for the first-pass review of this pull request. Thanks!

kevinlee12

thanks @Hudda, I also had 2 more cases that we should cover, please also add more if you can think of anymore. ptal!

scripts/pylint_extensions_test.py

codecov · 2019-11-06T06:38:42Z

Codecov Report

Merging #7929 into develop will decrease coverage by <.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #7929      +/-   ##
===========================================
- Coverage    83.95%   83.95%   -<.01%     
===========================================
  Files         1150     1150              
  Lines        68641    68641              
  Branches      3933     3933              
===========================================
- Hits         57622    57621       -1     
- Misses        9667     9668       +1     
  Partials      1352     1352

Flag	Coverage Δ
#backend	`100% <ø> (ø)`	⬆️
#frontend	`73.62% <ø> (ø)`	⬇️

Impacted Files	Coverage Δ
...mplates/dev/head/services/id-generation.service.ts	`83.33% <0%> (-16.67%)`	⬇️

kevinlee12

thanks @Hudda, I have more comments, ptal! By the way, once you are done addressing my comments, please ping me and/or assign this PR back to me.

kevinlee12 · 2019-11-07T04:07:45Z

scripts/pylint_extensions.py

+    """Checker for single space above args in python doc string."""
+
+    __implements__ = interfaces.IRawChecker
+    name = 'single-space-above-args'


single-space-above-args-raises-returns

kevinlee12 · 2019-11-07T04:10:02Z

scripts/pylint_extensions_test.py

+
+class SingleNewlineAboveArgsCheckerTests(unittest.TestCase):
+
+    def test_checks_newline_above_docstring(self):


Split up this test and more tests with varying combinations of args/returns/raises with and without spaces. (There should be at least 8 cases to cover all of them).

You still haven't split up the tests yet. Specifically, what I mean is to break this method up so that you have multiple def test_....

Hudda · 2019-11-07T05:42:55Z

@kevinlee12 ptal! and how do I assign you?

kevinlee12 · 2019-11-08T04:19:48Z

it looks like you don't have permissions to do so. for the future, just ping me 😄

kevinlee12 · 2019-11-08T04:21:46Z

@Hudda I've left a comment for you!

Hudda · 2019-11-12T03:19:46Z

@kevinlee12 ptal!

kevinlee12

lgtm, thanks @Hudda!

Added lint check for newline above args

5146994

Hudda requested a review from kevinlee12 as a code owner November 6, 2019 03:46

kevinlee12 self-assigned this Nov 6, 2019

kevinlee12 added CHANGELOG: Development workflow labels Nov 6, 2019

kevinlee12 suggested changes Nov 6, 2019

View reviewed changes

scripts/pylint_extensions_test.py Show resolved Hide resolved

kevinlee12 assigned Hudda and unassigned kevinlee12 Nov 6, 2019

Added more tests

83a2d38

Hudda requested a review from kevinlee12 November 6, 2019 06:40

kevinlee12 suggested changes Nov 7, 2019

View reviewed changes

Added lint check for newline above args

484c110

Hudda added 2 commits November 11, 2019 21:31

Merge remote-tracking branch 'upstream/develop' into newline

7779932

Split test in modules

fbb4dba

kevinlee12 approved these changes Nov 12, 2019

View reviewed changes

kevinlee12 unassigned Hudda Nov 12, 2019

kevinlee12 added the PR: LGTM label Nov 12, 2019

kevinlee12 merged commit cb99e89 into oppia:develop Nov 12, 2019

Hudda deleted the newline branch November 12, 2019 05:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #7868: Added lint check for a newline above args in python doc string #7929

Fix #7868: Added lint check for a newline above args in python doc string #7929

Hudda commented Nov 6, 2019 •

edited

Loading

oppiabot bot commented Nov 6, 2019

oppiabot bot commented Nov 6, 2019

kevinlee12 left a comment

codecov bot commented Nov 6, 2019 •

edited

Loading

kevinlee12 left a comment

kevinlee12 Nov 7, 2019

kevinlee12 Nov 7, 2019

Hudda Nov 7, 2019

kevinlee12 Nov 8, 2019

Hudda Nov 12, 2019

Hudda commented Nov 7, 2019 •

edited

Loading

kevinlee12 commented Nov 8, 2019

kevinlee12 commented Nov 8, 2019

Hudda commented Nov 12, 2019

kevinlee12 left a comment


		class SingleNewlineAboveArgsCheckerTests(unittest.TestCase):

		def test_checks_newline_above_docstring(self):

Fix #7868: Added lint check for a newline above args in python doc string #7929

Fix #7868: Added lint check for a newline above args in python doc string #7929

Conversation

Hudda commented Nov 6, 2019 • edited Loading

Explanation

Checklist

oppiabot bot commented Nov 6, 2019

oppiabot bot commented Nov 6, 2019

kevinlee12 left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 6, 2019 • edited Loading

Codecov Report

kevinlee12 left a comment

Choose a reason for hiding this comment

kevinlee12 Nov 7, 2019

Choose a reason for hiding this comment

kevinlee12 Nov 7, 2019

Choose a reason for hiding this comment

Hudda Nov 7, 2019

Choose a reason for hiding this comment

kevinlee12 Nov 8, 2019

Choose a reason for hiding this comment

Hudda Nov 12, 2019

Choose a reason for hiding this comment

Hudda commented Nov 7, 2019 • edited Loading

kevinlee12 commented Nov 8, 2019

kevinlee12 commented Nov 8, 2019

Hudda commented Nov 12, 2019

kevinlee12 left a comment

Choose a reason for hiding this comment

Hudda commented Nov 6, 2019 •

edited

Loading

codecov bot commented Nov 6, 2019 •

edited

Loading

Hudda commented Nov 7, 2019 •

edited

Loading