Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coala/docs: Add position number convention #5909

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bharatpurohit97
Copy link
Member

Added a note in Writing_Native_Bears.rst and a reminder
for developers in Writing_Linter_Bears.rst files.

Fixes #5297

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

  • I read the commit guidelines and I've followed
    them.
  • I ran coala over my code locally. (All commits have to pass
    individually.
    It is not sufficient to have "fixup commits" on your PR,
    our bot will still report the issues for the previous commit.) You will
    likely receive a lot of bot comments and build failures if coala does not
    pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

@@ -144,6 +144,10 @@ that is printed out.

For the exact list of named groups ``@linter`` recognizes, see the `API
documentation <https://api.coala.io/en/latest/>`__.
Pylint uses 0-based column convention and 1-based line convention.
We need to map linter's convention to coala's, please refer to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the between map and linter's

@@ -144,6 +144,10 @@ that is printed out.

For the exact list of named groups ``@linter`` recognizes, see the `API
documentation <https://api.coala.io/en/latest/>`__.
Pylint uses 0-based column convention and 1-based line convention.
We need to map linter's convention to coala's, please refer to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add so between comma and please

@@ -597,5 +597,7 @@ For example, let's make an aspect bear named SpellingCheckBear.
message='Wrong spelling in word `{}`'.format(word),
aspect=DictionarySpelling('py'),
)
.. note::
Coala uses 1-based line & column convention, i.e. the first line and first column are 1. However, some linter use 0-based convention. For example ``pylint`` uses 1-based line convention and 0-based column convention. We need to map these conventions, please refer to `Normalize line or Column convention <https://coala-api.netlify.com/developers/writing_linter_bears#normalize-line-or-column-numbers>`__.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coala should not be capitalized

@@ -597,5 +597,7 @@ For example, let's make an aspect bear named SpellingCheckBear.
message='Wrong spelling in word `{}`'.format(word),
aspect=DictionarySpelling('py'),
)
.. note::
Coala uses 1-based line & column convention, i.e. the first line and first column are 1. However, some linter use 0-based convention. For example ``pylint`` uses 1-based line convention and 0-based column convention. We need to map these conventions, please refer to `Normalize line or Column convention <https://coala-api.netlify.com/developers/writing_linter_bears#normalize-line-or-column-numbers>`__.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some linter -> some linters

@@ -597,5 +597,7 @@ For example, let's make an aspect bear named SpellingCheckBear.
message='Wrong spelling in word `{}`'.format(word),
aspect=DictionarySpelling('py'),
)
.. note::
Coala uses 1-based line & column convention, i.e. the first line and first column are 1. However, some linter use 0-based convention. For example ``pylint`` uses 1-based line convention and 0-based column convention. We need to map these conventions, please refer to `Normalize line or Column convention <https://coala-api.netlify.com/developers/writing_linter_bears#normalize-line-or-column-numbers>`__.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add so between comma and please

@@ -597,5 +597,7 @@ For example, let's make an aspect bear named SpellingCheckBear.
message='Wrong spelling in word `{}`'.format(word),
aspect=DictionarySpelling('py'),
)
.. note::
Coala uses 1-based line & column convention, i.e. the first line and first column are 1. However, some linter use 0-based convention. For example ``pylint`` uses 1-based line convention and 0-based column convention. We need to map these conventions, please refer to `Normalize line or Column convention <https://coala-api.netlify.com/developers/writing_linter_bears#normalize-line-or-column-numbers>`__.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why let people refer to linter bear doc? @linter is only for linter bears, and that does not help developers handle the situation in native bears.
Btw, you should use relative link when you refer to some other doc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@@ -144,6 +144,10 @@ that is printed out.

For the exact list of named groups ``@linter`` recognizes, see the `API
documentation <https://api.coala.io/en/latest/>`__.
Pylint uses 0-based column convention and 1-based line convention.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a reminder here is not very useful IMO. And the reminder does not bring any new information.
See #5297 (comment)

@@ -251,6 +251,9 @@ We need to map that to coala's convention as follows:
normalize_column_numbers = True,
...)

This is reminder for developers that they need to do research on linter bear,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a reminder my two cents.

@akshatkarani
Copy link
Member

@bharatpurohit97 This is not a bug, use closes instead of fixes.

Copy link
Member

@utkarsh2102 utkarsh2102 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Comments inline.
  • As @akshatkarani mentioned, please change Fixes to Closes.

@@ -251,6 +251,9 @@ We need to map that to coala's convention as follows:
normalize_column_numbers = True,
...)

This is reminder for developers that they need to do research on linter bear,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @frextrite suggested, it should be a reminder.
Plus, how about ... to do a proper research on ...?

@@ -598,4 +598,11 @@ For example, let's make an aspect bear named SpellingCheckBear.
aspect=DictionarySpelling('py'),
)

.. note::

coala uses 1-based line & column convention, i.e. the first line
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a comma after i.e., like: i.e.,.


coala uses 1-based line & column convention, i.e. the first line
and first column are 1. However, some linters use 0-based convention.
For example ``pylint`` uses 1-based line convention and 0-based column
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a comma after For example would make it look better.

@@ -251,6 +251,9 @@ We need to map that to coala's convention as follows:
normalize_column_numbers = True,
...)

This is a reminder for developers that they need to do a proper research on linter bear,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is longer than allowed. (88 > 80)

Origin: LineLengthBear, Section: docs.

@@ -251,6 +251,10 @@ We need to map that to coala's convention as follows:
normalize_column_numbers = True,
...)

This is a reminder for developers that they need to do a proper research on
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line contains following spacing inconsistencies:

  • Trailing whitespaces.

Origin: SpaceConsistencyBear, Section: docs.

The issue can be fixed by applying the following patch:

--- a/tmp/tmps0l_00tr/docs/Developers/Writing_Linter_Bears.rst
+++ b/tmp/tmps0l_00tr/docs/Developers/Writing_Linter_Bears.rst
@@ -251,7 +251,7 @@
             normalize_column_numbers = True,
             ...)
 
-This is a reminder for developers that they need to do a proper research on 
+This is a reminder for developers that they need to do a proper research on
 linter bear, ensure the convention and ideally write tests to ensure that 
 behaviour.
 

@@ -251,6 +251,10 @@ We need to map that to coala's convention as follows:
normalize_column_numbers = True,
...)

This is a reminder for developers that they need to do a proper research on
linter bear, ensure the convention and ideally write tests to ensure that
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line contains following spacing inconsistencies:

  • Trailing whitespaces.

Origin: SpaceConsistencyBear, Section: docs.

The issue can be fixed by applying the following patch:

--- a/tmp/tmps0l_00tr/docs/Developers/Writing_Linter_Bears.rst
+++ b/tmp/tmps0l_00tr/docs/Developers/Writing_Linter_Bears.rst
@@ -252,7 +252,7 @@
             ...)
 
 This is a reminder for developers that they need to do a proper research on 
-linter bear, ensure the convention and ideally write tests to ensure that 
+linter bear, ensure the convention and ideally write tests to ensure that
 behaviour.
 
 Suggest Corrections Using the ``corrected`` and ``unified-diff`` Output Formats

@@ -251,6 +251,10 @@ We need to map that to coala's convention as follows:
normalize_column_numbers = True,
...)

This is a reminder for developers that they need to do a proper research on
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line contains following spacing inconsistencies:

  • Trailing whitespaces.

Origin: SpaceConsistencyBear, Section: docs.

The issue can be fixed by applying the following patch:

--- a/tmp/tmps0l_00tr/docs/Developers/Writing_Linter_Bears.rst
+++ b/tmp/tmps0l_00tr/docs/Developers/Writing_Linter_Bears.rst
@@ -251,7 +251,7 @@
             normalize_column_numbers = True,
             ...)
 
-This is a reminder for developers that they need to do a proper research on 
+This is a reminder for developers that they need to do a proper research on
 linter bear, ensure the convention and ideally write tests to ensure that 
 behaviour.
 

@@ -251,6 +251,10 @@ We need to map that to coala's convention as follows:
normalize_column_numbers = True,
...)

This is a reminder for developers that they need to do a proper research on
linter bear, ensure the convention and ideally write tests to ensure that
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line contains following spacing inconsistencies:

  • Trailing whitespaces.

Origin: SpaceConsistencyBear, Section: docs.

The issue can be fixed by applying the following patch:

--- a/tmp/tmps0l_00tr/docs/Developers/Writing_Linter_Bears.rst
+++ b/tmp/tmps0l_00tr/docs/Developers/Writing_Linter_Bears.rst
@@ -252,7 +252,7 @@
             ...)
 
 This is a reminder for developers that they need to do a proper research on 
-linter bear, ensure the convention and ideally write tests to ensure that 
+linter bear, ensure the convention and ideally write tests to ensure that
 behaviour.
 
 Suggest Corrections Using the ``corrected`` and ``unified-diff`` Output Formats

@bharatpurohit97
Copy link
Member Author

@utkarsh2102 please re-review.

Copy link
Member

@utkarsh2102 utkarsh2102 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor change as suggested.
Else, looks good.

@@ -251,6 +251,10 @@ We need to map that to coala's convention as follows:
normalize_column_numbers = True,
...)

This is a reminder for developers that they need to do a proper research on
linter bear, ensure the convention and ideally write tests to ensure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be bears, no? 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess they should research particularly on linter bear

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more accurate, this should be linter instead of linter bear.

@@ -251,6 +251,10 @@ We need to map that to coala's convention as follows:
normalize_column_numbers = True,
...)

This is a reminder for developers that they need to do a proper research on
linter bear, ensure the convention and ideally write tests to ensure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more accurate, this should be linter instead of linter bear.

@@ -251,6 +251,10 @@ We need to map that to coala's convention as follows:
normalize_column_numbers = True,
...)

This is a reminder for developers that they need to do a proper research on
linter bear, ensure the convention and ideally write tests to ensure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use .. note:: here


coala uses 1-based line & column convention, i.e., the first line
and first column are 1. However, some linters use 0-based convention.
For example, ``pylint`` uses 1-based line convention and 0-based column
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bad example, as pylint is a linter, which has nothing to do with writing a native bear.
You might want to find an example among existing native bears

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found, there are some bears in coala like - PyLintBear , CheckstyleBear ,StylelintBear . they are normalized, but they all are linter bears.
@li-boxuan is there any native bear that use 0 based convention. Btw i removed linter example.

.. note::

coala uses 1-based line & column convention, i.e., the first line
and first column are 1. However, some linters use 0-based convention.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"linters" is not relevant for native bears where the Result objects are created in coala, and the Result constructor will raise an exception if '0' are given (and due to our 100% test coverage policy, all Result constructors will be tested).

Added note in Writing_Native_Bears.rst and a reminder
for developers in Writing_Linter_Bears.rst files.

Closes coala#5297
@bharatpurohit97
Copy link
Member Author

@jayvdb is it good to go now? please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Add position number convention to native/linter bear guide
10 participants