DartLintBear: check settings validity #950

NiklasMM · 2016-10-28T15:50:47Z

As pointed out here dart-lang/dart_style#261
dartanalyzer only supports indentation using 2 spaces. This commit
changes DartLintBear to check that use_spaces is True and
indent_size is 2 and raises a ValueError if not.

Fixes #897

gitmate-bot · 2016-10-28T15:51:01Z

Thanks for your contribution!

Reviewing pull requests take really a lot of time and we're all volunteers. Please make sure you go through the following check list and complete them all before pinging someone for a review.

I have rebased properly. Please see this for a tutorial.
I have gone through the commit guidelines and I've followed them:
- I have followed the guidelines for the commit shortlog.
- I have followed the guidelines for the commit body.
- I have included the issue URL.
- I have used the Fixes keyword if the commit fixes a real bug and the Closes keyword if it adds a feature/enhancement. See more here.
I have run all the tests and they all pass. That includes that all CI (below) is green - if GitMate issues appear you will have to amend your commits, pushing new commits on top is not sufficient!

As you learn things over your Pull Request please help others on the chat and on PRs to get their stuff right as well!

Adrianzatreanu · 2016-10-28T16:01:49Z

apparently this fails because it is not rebased, and does not contain the changes you fixed in the other branch. could you rebase it and i think all will pass :D ? thanks!

NiklasMM · 2016-10-28T17:17:56Z

Yeah, will do this tomorrow... My stupid hotel only allows HTTP(S) traffic =(

AbdealiLoKo · 2016-10-30T03:14:45Z

bears/dart/DartLintBear.py

+
+        # use_spaces must be True and indent_size must be 2 because
+        # dartanalyzer only supports these settings
+        # see https://github.com/dart-lang/dart_style/issues/261


Thank you for putting the link here ! Really appreciated :)

AbdealiLoKo · 2016-10-30T03:17:19Z

tests/dart/DartLintBearTest.py

+        section.append(Setting('use_spaces', False))
+        bear = DartLintBear(section, Queue())
+
+        with pytest.raises(AssertionError):


Why not use unittest's assertRaises ? https://docs.python.org/2/library/unittest.html#unittest.TestCase.assertRaises

It has the same functionality (The reason I mention is because it's nicer to use the builtin unittest as much as possible)

Well, since py.test is used as a test runner, I thought it best to use it directly as much as possible. But if it serves the uniformity of the code base, I'll change it ;)

AbdealiLoKo · 2016-10-30T03:18:42Z

tests/dart/DartLintBearTest.py

@@ -30,3 +37,23 @@
                                     valid_files=(good_file,),
                                     invalid_files=(bad_file,),
                                     tempfile_kwargs={"suffix": ".dart"})
+
+
+@skipIf(which('dartanalyzer') is None, 'dartanalyzer is not installed')


Don't write your custom check please !
Use the skip generator for the bear ? https://github.com/coala/coala-bears/blob/master/tests/BearTestHelper.py#L4

NiklasMM · 2016-10-30T10:36:16Z

@AbdealiJK Thanks for the review! I made the requested changes.

Adrianzatreanu · 2016-10-30T10:48:17Z

bears/dart/DartLintBear.py

+        # see https://github.com/dart-lang/dart_style/issues/261
+        if (indent_size != 2 or not use_spaces):
+            raise ValueError(
+                "dartanazlyer only supports use_space=True and indent_size=2")


typo: dartanalyzer here :)

Adrianzatreanu · 2016-10-30T10:48:55Z

bears/dart/DartLintBear.py

+        # see https://github.com/dart-lang/dart_style/issues/261
+        if (indent_size != 2 or not use_spaces):
+            raise ValueError(
+                "dartanazlyer only supports use_space=True and indent_size=2")


also, an user probably doesnt know what dartanalyzer is, as we are wrapping it into a bear, im not 100% percent, but maybe we should say:
DartLintBear only supports.. ? what do you think?

Adrianzatreanu · 2016-10-30T10:49:49Z

tests/dart/DartLintBearTest.py

+        bear = DartLintBear(section, Queue())
+
+        with self.assertRaises(AssertionError):
+            self.check_validity(bear, [], good_file)


this tests are actually nice work :)

AbdealiLoKo · 2016-10-30T10:55:45Z

@NiklasMM Nearly there. Just a few minor things and a rebase

sils · 2016-10-30T13:17:45Z

bears/dart/DartLintBear.py

+        # see https://github.com/dart-lang/dart_style/issues/261
+        if (indent_size != 2 or not use_spaces):
+            raise ValueError(
+                "DartLintBear only supports use_space=True and indent_size=2")


use_space -> use_spaces, I'd also put the "ini extracts" in backticks

As pointed out here dart-lang/dart_style#261, dartanalyzer only supports indentation using 2 spaces. This commit changes DartLintBear to check that ``use_spaces`` is True and ``indent_size`` is 2 and raises a ``ValueError`` if not. Fixes #897

sils · 2016-10-30T13:31:26Z

907b477 is ready

changes have been addressed

sils · 2016-10-30T14:28:14Z

@rultor merge

sils · 2016-10-30T14:28:23Z

Hey, this is your first contribution, right? Congrats and welcome to the coalaians! :)

You can help us a lot by filling out http://coala.io/newform!

Rultor will now automatically release your changes as a prerelease so users can experience the goodness of your changes right now.

See also https://github.com/coala-analyzer/coala/wiki/coala-Steam-Offer btw.

We would really appreciate it if you could help us with

some code reviews for other newcomers
fixing at least a difficulty/low issue

We're all volunteers and we're currently struggling to keep this up. Helping all those newcomers is a lot of work and we do need your help so we can continue this for others!

rultor · 2016-10-30T14:28:52Z

@rultor merge

@sils OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2016-10-30T14:30:15Z

@rultor merge

@sils Done! FYI, the full log is here (took me 1min)

gitmate-bot added process/pending review size/S labels Oct 28, 2016

Adrianzatreanu added process/wip and removed process/pending review labels Oct 28, 2016

gitmate-bot added process/pending review and removed process/wip labels Oct 29, 2016

AbdealiLoKo previously requested changes Oct 30, 2016

View reviewed changes

AbdealiLoKo added process/wip and removed process/pending review labels Oct 30, 2016

gitmate-bot added process/pending review and removed process/wip labels Oct 30, 2016

Adrianzatreanu reviewed Oct 30, 2016

View reviewed changes

Adrianzatreanu added process/wip and removed process/pending review labels Oct 30, 2016

gitmate-bot added process/pending review and removed process/wip labels Oct 30, 2016

sils reviewed Oct 30, 2016

View reviewed changes

sils added process/wip and removed process/pending review labels Oct 30, 2016

gitmate-bot added process/pending review and removed process/wip labels Oct 30, 2016

DartLintBear: Check settings validity

907b477

As pointed out here dart-lang/dart_style#261, dartanalyzer only supports indentation using 2 spaces. This commit changes DartLintBear to check that ``use_spaces`` is True and ``indent_size`` is 2 and raises a ``ValueError`` if not. Fixes #897

sils added process/approved and removed process/pending review labels Oct 30, 2016

rultor merged commit 907b477 into coala:master Oct 30, 2016

NiklasMM mentioned this pull request Nov 5, 2016

Replace pull request https://github.com/coala/coala-bears/pull/951, which had a history beyond saving, because it was done during the pycon sprint. #974

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DartLintBear: check settings validity #950

DartLintBear: check settings validity #950

NiklasMM commented Oct 28, 2016

gitmate-bot commented Oct 28, 2016

Adrianzatreanu commented Oct 28, 2016

NiklasMM commented Oct 28, 2016

AbdealiLoKo Oct 30, 2016

AbdealiLoKo Oct 30, 2016

NiklasMM Oct 30, 2016

AbdealiLoKo Oct 30, 2016

NiklasMM commented Oct 30, 2016

Adrianzatreanu Oct 30, 2016

Adrianzatreanu Oct 30, 2016

AbdealiLoKo Oct 30, 2016

Adrianzatreanu Oct 30, 2016

AbdealiLoKo commented Oct 30, 2016

sils Oct 30, 2016

sils commented Oct 30, 2016

sils commented Oct 30, 2016

sils commented Oct 30, 2016

rultor commented Oct 30, 2016

rultor commented Oct 30, 2016

DartLintBear: check settings validity #950

DartLintBear: check settings validity #950

Conversation

NiklasMM commented Oct 28, 2016

gitmate-bot commented Oct 28, 2016

Adrianzatreanu commented Oct 28, 2016

NiklasMM commented Oct 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NiklasMM commented Oct 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AbdealiLoKo commented Oct 30, 2016

Choose a reason for hiding this comment

sils commented Oct 30, 2016

sils commented Oct 30, 2016

sils commented Oct 30, 2016

rultor commented Oct 30, 2016

rultor commented Oct 30, 2016