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

Improve pyrex cython linter. #1675

Merged
merged 3 commits into from Jun 27, 2018

Conversation

Projects
None yet
2 participants
@nicopauss
Copy link
Contributor

commented Jun 25, 2018

Like many other linters, use variables for the executable and options
used by the linter.
By default, the linter now report every warnings as errors with
--warning-errors.
Also add include directory and set working directory to file directory.

Improve pyrex cython linter.
Like many other linters, use variables for the executable and options
used by the linter.
By default, the linter now report every warnings as errors with
`--warning-errors`.
Also add include directory and set working directory to file directory.

@nicopauss nicopauss force-pushed the nicopauss:master branch from 304fefd to bedd30e Jun 25, 2018

" Description: cython syntax checking for cython files.

call ale#Set('pyrex_cython_executable', 'cython')
call ale#Set('pyrex_cython_options', '--warning-extra --warning-errors')

This comment has been minimized.

Copy link
@w0rp

w0rp Jun 25, 2018

Owner

Could you remove --warning-errors here, so we can preserve the current behavior? If users want warnings to be errors, they can change the option themselves. I don't want to change the behavior without a very compelling reason, and whether warnings should be errors is subjective.

This comment has been minimized.

Copy link
@nicopauss

nicopauss Jun 25, 2018

Author Contributor

Done.

Handle cython warning with custom handle and remove '--warning-errors'.
Add a custom handler to support cython warning format.
Remove '--warning-errors' to keep previous behaviour.
@w0rp
Copy link
Owner

left a comment

Cool, that'll be more agreeable for people. Now add a test for the GetCommand function. Have a look at tests in test/command_callback, maybe at the tests for Java and a few other things.

Add test_pyrex_cython_command_callback.vader
Add common callback tests to check if executable and options are well
configurable.

@nicopauss nicopauss force-pushed the nicopauss:master branch from 460dfec to 4d935ff Jun 25, 2018

@nicopauss

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2018

Done

@w0rp

w0rp approved these changes Jun 27, 2018

@w0rp

This comment has been minimized.

Copy link
Owner

commented Jun 27, 2018

Good stuff. 👍 Cheers! 🍻

@w0rp w0rp merged commit 980aa35 into w0rp:master Jun 27, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.