FormatRBear.py: disable parameter-options #1335

yogupta · 2017-01-21T13:16:50Z

r_keep_comments,r_keep_blank_lines,r_braces_on_next_line,r_use_arrows are
disabled and will not be checked untill the user provides value.

closes #514

gitmate-bot · 2017-01-21T13:17:05Z

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!

gitmate-bot · 2017-01-21T13:17:52Z

bears/r/FormatRBear.py

+            options.add('comment=' + _map_to_r_bool(r_keep_comments))
+        if r_use_arrows is not None:
+            options.add('arrow=' + _map_to_r_bool(r_use_arrows))
+


W293 blank line contains whitespace'

PycodestyleBear (W293), severity NORMAL, section autopep8.

gitmate-bot · 2017-01-21T13:17:52Z

bears/r/FormatRBear.py

                   'indent=' + str(indent_size)}
        if r_max_expression_length:
            options.add('width.cutoff=' + str(r_max_expression_length))
-
+        if r_braces_on_next_line is not None:
+            options.add('brace.newline=' + _map_to_r_bool(r_braces_on_next_line))


E501 line too long (81 > 79 characters)'

PycodestyleBear (E501), severity NORMAL, section autopep8.

gitmate-bot · 2017-01-21T13:17:53Z

bears/r/FormatRBear.py

+            options.add('comment=' + _map_to_r_bool(r_keep_comments))
+        if r_use_arrows is not None:
+            options.add('arrow=' + _map_to_r_bool(r_use_arrows))
+


The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/bears/r/FormatRBear.py +++ b/bears/r/FormatRBear.py @@ -89,7 +89,7 @@ options.add('comment=' + _map_to_r_bool(r_keep_comments)) if r_use_arrows is not None: options.add('arrow=' + _map_to_r_bool(r_use_arrows)) - + rcode = 'library(formatR);formatR::tidy_source({})'.format( ','.join(options)) return '-e', rcode

gitmate-bot · 2017-01-21T13:17:53Z

bears/r/FormatRBear.py

                   'indent=' + str(indent_size)}
        if r_max_expression_length:
            options.add('width.cutoff=' + str(r_max_expression_length))
-
+        if r_braces_on_next_line is not None:
+            options.add('brace.newline=' + _map_to_r_bool(r_braces_on_next_line))


The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/bears/r/FormatRBear.py +++ b/bears/r/FormatRBear.py @@ -82,7 +82,8 @@ if r_max_expression_length: options.add('width.cutoff=' + str(r_max_expression_length)) if r_braces_on_next_line is not None: - options.add('brace.newline=' + _map_to_r_bool(r_braces_on_next_line)) + options.add('brace.newline=' + + _map_to_r_bool(r_braces_on_next_line)) if r_keep_blank_lines is not None: options.add('blank=' + _map_to_r_bool(r_keep_blank_lines)) if r_keep_comments is not None:

gitmate-bot · 2017-01-21T13:17:54Z

bears/r/FormatRBear.py

                   'indent=' + str(indent_size)}
        if r_max_expression_length:
            options.add('width.cutoff=' + str(r_max_expression_length))
-
+        if r_braces_on_next_line is not None:
+            options.add('brace.newline=' + _map_to_r_bool(r_braces_on_next_line))


Line is longer than allowed. (81 > 79)

LineLengthBear, severity NORMAL, section linelength.

gitmate-bot · 2017-01-21T13:17:55Z

Comment on b00f3bd.

Body of HEAD commit contains too long lines. Commit body lines should not exceed 72 characters.

GitCommitBear, severity NORMAL, section commit.

gitmate-bot · 2017-01-21T13:17:55Z

bears/r/FormatRBear.py

+            options.add('comment=' + _map_to_r_bool(r_keep_comments))
+        if r_use_arrows is not None:
+            options.add('arrow=' + _map_to_r_bool(r_use_arrows))
+


Line contains following spacing inconsistencies:

Trailing whitespaces.

SpaceConsistencyBear, severity NORMAL, section python.

The issue can be fixed by applying the following patch:

--- a/bears/r/FormatRBear.py +++ b/bears/r/FormatRBear.py @@ -89,7 +89,7 @@ options.add('comment=' + _map_to_r_bool(r_keep_comments)) if r_use_arrows is not None: options.add('arrow=' + _map_to_r_bool(r_use_arrows)) - + rcode = 'library(formatR);formatR::tidy_source({})'.format( ','.join(options)) return '-e', rcode

gitmate-bot · 2017-01-21T13:17:56Z

Comment on b00f3bd.

Shortlog of HEAD commit does not match given regex: ([^:]|[^:]+: [A-Z0-9].*)

GitCommitBear, severity NORMAL, section commit.

gitmate-bot · 2017-01-21T14:58:21Z

Comment on a4de771.

Shortlog of HEAD commit does not match given regex: ([^:]|[^:]+: [A-Z0-9].*)

GitCommitBear, severity NORMAL, section commit.

Udayan12167 · 2017-01-21T15:00:28Z

Hey your shortlog is greater than 50 characters try shortening it.

pratyushprakash · 2017-01-21T15:53:12Z

bears/r/FormatRBear.py

+        if r_keep_comments is not None:
+            options.add('comment=' + _map_to_r_bool(r_keep_comments))
+        if r_use_arrows is not None:
+            options.add('arrow=' + _map_to_r_bool(r_use_arrows))


I think is not None is redundant here.

Same logic goes here also . user may have used both = and -> . if it doesn't check is not None then it will skip if statement.
Suppose user provided False value.then it will skip if and the values won't effect the code.
Correct me if i'm wrong

In this case both True and False will add the option

if i keep it None while declaring then , False value will never add up into options

@IncorrectUsername If r_keep_comments is set to False in settings the line options.add('comment=' + _map_to_r_bool(r_keep_comments)) will be executed.

Yes , it will be executed.
I was saying that if you don't provide value in settings then it won't be executed.

i am just wondering ( i don't have formtR installled )
what is wrong with this piece of code given that all of the settings have been disabled

1+1 if(TRUE){ x=1 # inline comments }else{x=2;print("Oh no... ask the right bracket to go away!")}

Unless i am not getting this wrong @Makman2, but the problem here is that these settings are too specific right, and we want the user to enable them manualy, given that the file above is not correct, then something is not right @IncorrectUsername, check on that would you ?

pratyushprakash · 2017-01-21T15:53:57Z

bears/r/FormatRBear.py

+                         r_keep_comments: bool=None,
+                         r_keep_blank_lines: bool=None,
+                         r_braces_on_next_line: bool=None,
+                         r_use_arrows: bool=None,


This should be False instead of None.

If it is False then ,suppose for eg: r_use_arrows=false then there will be no arrows everything will be equal(=) .

It was right (using None). The original issue of this PR is making FormatRBear ignore those option. Meaning they dont enforce using arrow (True) neither never user arrow (False).

pratyushprakash · 2017-01-21T22:00:33Z

@IncorrectUsername Could you add tests for this ?

yogupta · 2017-01-23T13:34:45Z

@pratyushprakash I'm sorry . I could not add tests , I'm getting error Rscript is not installed :(

gitmate-bot · 2017-01-30T14:26:02Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-02-01T10:43:02Z

bears/haskell/GhcModBear.py

@@ -1,7 +1,8 @@
 from coalib.bearlib.abstractions.Linter import linter
 from dependency_management.requirements.CabalRequirement import (
    CabalRequirement)
-
+from dependency_management.requirements.DistributionRequirement import (
+    DistributionRequirement)

 @linter(executable='ghc-mod',


E302 expected 2 blank lines, found 1'

PycodestyleBear (E302), severity NORMAL, section autopep8.

gitmate-bot · 2017-02-01T10:43:03Z

bears/haskell/GhcModBear.py

@@ -1,7 +1,8 @@
 from coalib.bearlib.abstractions.Linter import linter
 from dependency_management.requirements.CabalRequirement import (
    CabalRequirement)
-
+from dependency_management.requirements.DistributionRequirement import (
+    DistributionRequirement)


The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/bears/haskell/GhcModBear.py +++ b/bears/haskell/GhcModBear.py @@ -3,6 +3,7 @@ CabalRequirement) from dependency_management.requirements.DistributionRequirement import ( DistributionRequirement) + @linter(executable='ghc-mod', output_format='regex',

gitmate-bot · 2017-02-15T18:28:07Z

tests/r/FormatRBearTest.py

+
+FormatRBearDefaultSettingsTest = verify_local_bear(
+    FormatRBear,
+    valid_files=(good_file_7,good_file_4),


The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/tests/r/FormatRBearTest.py +++ b/tests/r/FormatRBearTest.py @@ -142,6 +142,6 @@ FormatRBearDefaultSettingsTest = verify_local_bear( FormatRBear, - valid_files=(good_file_7,good_file_4), + valid_files=(good_file_7, good_file_4), invalid_files=(bad_file_3,), settings={'r_keep_blank_lines': 'true'})

Makman2 · 2017-02-15T22:32:02Z

tests/r/FormatRBearTest.py

+    FormatRBear,
+    valid_files=(good_file_7, good_file_4),
+    invalid_files=(bad_file_3,),
+    settings={'r_keep_blank_lines': 'true'})


as it's a default test, you maybe want to omit settings^^ (especially r_keep_blank_lines doesn't do anything in fact :3)

Makman2 · 2017-02-15T22:33:31Z

Hm seems tests are still failing, did you try them locally? (If you don't want to install R and anything needed for coala, you can grab our latest docker image)

yogupta · 2017-02-16T15:20:19Z

@Makman2 after installing image on docker and running py.test -k FormatRBearTest the tests are skipping.

Makman2 · 2017-02-16T16:17:03Z

Tests do not skip for me, and when I checkout your branch I get the same error like in the CI. Did you run the command in the container actually?

yogupta · 2017-02-19T06:52:46Z

@Makman2 I followed your steps and the tests for the FormatRBearTest passed. check it here asciinema. but still circle is failing.:(

and then when i ran only py.test the tests were failing for other modules.

Makman2 · 2017-02-20T17:21:44Z

rebase and repush, maybe it's a temporary issue^^

nemani · 2017-02-27T21:34:54Z

@Makman2 Can you restart the travis?
Feels a bit weird failing on one version of python.

Makman2 · 2017-02-27T21:40:25Z

build restarted 👍

supergr35 · 2017-03-01T15:18:59Z

By the way, advice for the future: Please don't develop on the master branch. It can create some problems down the line, and we develop on other branches. If you don't know how check the git basics documentation. :)

Makman2 · 2017-03-01T15:56:59Z

ack ba0c25c

Makman2 · 2017-03-01T15:57:04Z

rebase and lgtm ;)

Makman2 · 2017-03-02T17:56:22Z

reack c8810e3

Default options of r_braces_on_next_line, r_use_arrows are disabled. Closes #514

jayvdb · 2017-03-06T10:02:54Z

reack ae985fd

jayvdb · 2017-03-06T10:03:08Z

@rultor merge

rultor · 2017-03-06T10:04:29Z

@rultor merge

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

rultor · 2017-03-06T10:06:11Z

@rultor merge

@jayvdb Done! FYI, the full log is here (took me 2min)

gitmate-bot added process/pending review size/XS review carefully! labels Jan 21, 2017

gitmate-bot reviewed Jan 21, 2017

View reviewed changes

Udayan12167 added process/wip and removed process/pending review labels Jan 21, 2017

gitmate-bot added process/pending review and removed process/wip labels Jan 21, 2017

pratyushprakash suggested changes Jan 21, 2017

View reviewed changes

Udayan12167 added process/wip and removed process/pending review labels Jan 22, 2017

gitmate-bot added process/pending review size/S and removed process/wip size/XS labels Feb 1, 2017

gitmate-bot reviewed Feb 1, 2017

View reviewed changes

gitmate-bot reviewed Feb 15, 2017

View reviewed changes

Makman2 reviewed Feb 15, 2017

View reviewed changes

coala-bot added process/wip and removed process/pending review labels Feb 16, 2017

gitmate-bot added process/pending review and removed process/wip labels Feb 18, 2017

coala-bot added process/wip and removed process/pending review labels Feb 19, 2017

coala-bot added process/pending review and removed process/wip labels Feb 19, 2017

Makman2 added process/approved and removed process/pending review labels Mar 2, 2017

gitmate-bot added the process/pending review label Mar 3, 2017

FormatRBear.py: Disable parameter-options

ae985fd

Default options of r_braces_on_next_line, r_use_arrows are disabled. Closes #514

rultor merged commit ae985fd into coala:master Mar 6, 2017

FormatRBear.py: disable parameter-options #1335

FormatRBear.py: disable parameter-options #1335

Conversation

yogupta commented Jan 21, 2017

gitmate-bot commented Jan 21, 2017

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

Choose a reason for hiding this comment

gitmate-bot commented Jan 21, 2017

Choose a reason for hiding this comment

gitmate-bot commented Jan 21, 2017

gitmate-bot commented Jan 21, 2017

Udayan12167 commented Jan 21, 2017

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

pratyushprakash Jan 21, 2017 • edited Loading

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

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

pratyushprakash commented Jan 21, 2017

yogupta commented Jan 23, 2017 • edited Loading

gitmate-bot commented Jan 30, 2017

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

Makman2 commented Feb 15, 2017

yogupta commented Feb 16, 2017

Makman2 commented Feb 16, 2017

yogupta commented Feb 19, 2017

Makman2 commented Feb 20, 2017

nemani commented Feb 27, 2017

Makman2 commented Feb 27, 2017

supergr35 commented Mar 1, 2017

Makman2 commented Mar 1, 2017

Makman2 commented Mar 1, 2017

Makman2 commented Mar 2, 2017

jayvdb commented Mar 6, 2017

jayvdb commented Mar 6, 2017

rultor commented Mar 6, 2017

rultor commented Mar 6, 2017

pratyushprakash Jan 21, 2017 •

edited

Loading

yogupta commented Jan 23, 2017 •

edited

Loading