restLintBear: Ignore unknown errors #1718

SanketDG · 2017-05-08T22:53:09Z

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:

pick up any PR at https://coala.io/review
review it (check https://coala.io/reviewing for more info)
if you are sure that it needs work, use cobot mark wip <URL> to get it out
of the review queue.

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

Fixes #1716

SanketDG · 2017-05-08T22:53:46Z

needs better test names 😆

jayvdb · 2017-05-09T01:10:10Z

bears/rest/reSTLintBear.py

@@ -14,13 +28,17 @@ class reSTLintBear(LocalBear):
    LICENSE = 'AGPL-3.0'
    CAN_DETECT = {'Formatting', 'Syntax'}

-    def run(self, filename, file):
+    def run(self, filename, file, ignore_unknown_roles: bool=False):


directives are not roles, so either this is ignore_unknown: bool, or it is split into ignore_unknown_roles and ignore_unknown_directives.

Even if we did plan on deprecating this bear, this PR still makes sense, as it will help us implement the bear, and thus understand the bear ;-)

gah I am not sure if we deprecate the bear, it will be worth doing this.

But now that I have seen rstcheck's way of doing this, I would also like to have two params ignore_directives, ignore_roles, which takes in a list of directives/roles to ignore. And then obviously do the stuff with regex and the helper function.

I dont see the benefit of replicating rstcheck , unless we know of a feature in restlint that is missing from rstcheck...?

I am not really sure, there is no feature "unimplemented" in restlint because both uses docutils

However, I feel restlint is the "purer" form of a rst linter, because rstcheck has "batteries" included like sphinx support etc. This is the only argument I have,

That said, I am up for deprecating restlint for the sake of bear uniformity.

c.f. #1723

Also see coala/coala#3044 (comment) , where I think I saw extra error messages from restlint than rstcheck emitted .

I think you might be right about the need for a purer rst linter. https://pypi.python.org/pypi/restructuredtext_lint reminds me ... we need a rst-checker which validates README.rst so that it can be used for PyPI, which has stricter rules than Sphinx, and stricter rules than GitHub's RST parser.

Maybe we can request rstcheck has a strict rst mode. Oddly enough, this coala project seemed like it might have created a more clear separation between docutils mode and sphinx mode.

And if we need a pure rst linter, then we dont want to allow it to validate sphinx rst. It should fail! ;-)
And we should implement restLintBear on any README.rst or other file which is included in the packaging for PyPI, and not other RST files.

So close this PR then seems the right way?

gitmate-bot · 2017-05-22T21:30:35Z