markstory/lint-review

An automated code linting bot that integrates various code lint tools with github pull requests.

Lint Review

Lint Review helps automate a tedious part of code review - enforcing coding standards. By using the GitHub API Lint Review runs a repository's configured linters and updates pull requests with line comments where lint errors would be introduced.

Lint Review requires:

• Python 3.6+
• RabbitMQ (or any other Message broker that is compatible with Celery)
• A publically addressable hostname/IP that either GitHub or your github:enterprise can reach.
• A GitHub account with read/write access to the repositories you want linted. This account is used to post comments on pull reviews.
• Docker as all tools are executed in containers.

Lint Review runs as two processes. A web process handles accepting webhooks from github, and a celery process handles cloning repositories and running containers for lint tools.

Hosted Lint Reviews

If you don't want to go through the trouble of setting up your own installation of lint-review, stickler-ci.com provides a hosted version of lint-review featuring all the linters installed, and an easier to use YAML config file.

In the free plan, Stickler CI provides the following for open source projects:

• Hosted service
• Connection to public repositories
• Commenting on style errors
• Auto fixing for style errors
• Clean user interface
• Robust documentation
• Automatic upgrades for linting tools.

A paid plan of Stickler CI allows you to enable private repositories and fine grained access control to repositories via the GitHubApp integration.

Installation

You install Lint Review by cloning the repository and installing dependecies, or by using docker. If you are not using docker, it is recommended that you use virtualenv to save shaving yaks down the road.

git clone git://github.com/markstory/lint-review.git
cd lint-review
virtualenv env
source env/bin/activate
pip install .

Make docker images for tools you want to use

In addition to installing the dependencies for lint-review you will also need to build the containers for each lint tool you want to use:

# Create a set of images for frequently used tools.
make images

Once the dependencies are installed you should configure the repositories you want to review.

Running lintreview services in Docker

To use docker, you'll need to install both docker, docker-compose and possibly docker toolbox depending on your operating system. Once you have the docker installed, you can boot up lint-review into docker using:

docker-compose up -d broker worker web

Edit docker-compose.yml and customise your configuration by setting keys under environment for the web and worker processes. For the most basic installation you'll need to set GITHUB_OAUTH_TOKEN, and LINTREVIEW_SERVER_NAME.

Lint Review Configuration

Lint review is configured through a settings file. Both the web app and celery process share the same configuration file, so configuration is easier to manage and share.

• Copy the settings.sample.py to settings.py

• Edit the required configuration options, or set the correct environment variables.

• Set the LINTREVIEW_SETTINGS environment variable to the path of your configuration files. In *nix system this can be done via:

  export LINTREVIEW_SETTINGS='/path/to/settings.py'
  

• You can skip setting LINTREVIEW_SETTINGS if you're running lintreview from a directory containing your settings.py file.

You can also have per install configuration files by defining the LINTRC_DEFAULTS config option in your settings file. This file should be a .lintrc config file. It will be merged with each projects .lintrc before running tools. This gives you an easy way to have global configuration for tools.

Setting up Repositories

Once you've configured the server processes, it is time to setup some repositories to be checked.

Installing GitHub hooks

Before Lint Review can check pull requests on a repository webhooks will need to be installed. You can install webhooks by running the built-in command line tool:

source env/bin/activate
lintreview register mark awesome-stuff

Or, if you're using Docker:

docker-compose run web lintreview register mark awesome-stuff

The above register webhooks for the given user & repository. You can use the --user and --password options to provide the repository admin credentials if the user lint-review runs as does not have admin access to the repository. You can also use the cli tool to remove webhooks:

source env/bin/activate
lintreview unregister mark awesome-stuff

Warning The current web server name will be registered with github. Make sure it is configured properly before registering hooks, or you'll need to remove any registered hooks and start over.

.lintrc files

Lint Review use hidden ini files to configure the tools used on each project. The .lintrc file defines the various linting tools and any arguments for each one. Lint tools must be tools Lint Review knows about. See lint tools for available tools. A sample .lintrc file would look like.

[files]
ignore = generated/*
    vendor/*

[tools]
linters = pep8, jshint

[tool_pep8]
ignore = W2,E401

[tool_jshint]
config = path/to/jshint.json

The [tools] section is required, and linters should be a list of linters your project uses. Each tool can also have a section prefixed with tool_ to define additional configuration options for each tool. The documentation for each tool outlines which options are supported.

The [files] section is optional and allows you to define ignore patterns. These patterns are used to find and exclude files when doing a review. Ignore patterns use glob expressions to find files. The patterns start at the reviewed repository root. If you need to ignore mulitple patterns separate them with new lines.

Running Lint Review

After setting up configuration you'll need to start up both processes:

source env/bin/activate
gunicorn -c settings.py lintreview.web:app
celery -A lintreview.tasks worker

Now when ever a pull request is opened or updated for a registered repository new jobs will be spun up and lint will be checked and commented on.

Lint tools

Python:

Flake8

Uses the flake8 module to check code.

Options

• ignore Set which pep8 error codes you wish to ignore.
• exclude Exclude files matching these comma separated patterns (default: .svn, CVS, .bzr, .hg, .git)
• filename When parsing directories, only check filenames matching these comma separated patterns (default: *.py)
• select Select errors and warnings (e.g. E,W6)
• max-line-length Set maximum allowed line length (default: 79)
• format Set the error format [default|pylint|]
• max-complexity McCabe complexity threshold
• snippet Interacts with flake8-snippets allowing you to trigger errors on specific snippets you want to disallow.

These options are passed into flake8 as cli options.

pep8

Uses the pep8 module to check code.

Options

• ignore Set which pep8 error codes you wish to ignore.
• exclude Exclude files or directories which match these comma separated patterns (default: .svn, CVS, .bzr, .hg, .git)
• filename When parsing directories, only check filenames matching these comma separated patterns (default: *.py)
• select Select errors and warnings (e.g. E,W6)
• max-line-length Set maximum allowed line length (default: 79)

py3k

Uses pylint to check for Python 3 compatibility issues.

Options

• ignore A comma separated list of error codes you wish to ignore.
• ignore-patterns A comma separated list of regular expressions that match files you want to ignore.

pytype

Uses pytype to check python code using type annotations.

Options

• config Path to a config file for pytype.
• fixer Enable automatic merging of inferred pyi files.

PHP

PHPCS

Uses the phpcs PEAR library to do style checks on PHP, Javascript and or CSS files.

Options

• standard The coding standard to use. By default the PSR2 standard is used. You can use any of the built-in standards or provide your own inside your project directory.
• extensions The extensions to check. By default only .php files will be checked.
• ignore A glob path of files to ignore.
• exclude A comma separated list of sniffs to not apply.
• tab_width The number of spaces to convert tabs into, this is useful for projects using tabs for indentation.

PHPMD

Uses the phpmd tool to check code for messy or hard to maintain code.

Options

• ruleset The ruleset names or file to use for phpmd. Defaults to cleancode,codesize,unusedcode

Javascript:

JSHint

Uses the jshint npm module to check javascript files. Before you can use this linter you'll need to install nodejs and the jshint npm package:

cd path/to/lintreview
npm install jshint

Options

• config Provide a path to the json config file for jshint.

ESLint

Uses the eslint npm module to check javascript files. Before you can use this linter you'll need to install the eslint npm package:

cd path/to/lintreview
npm install eslint

Options

• config Provide a path to the json config file for eslint.

StandardJs

Uses the standard npm module to check javascript files. Before you can use this linter you'll need to install nodejs and the standard npm package:

Options

• None currently supported

JSON Lint

Uses the jsonlint script from demjson python module to check javascript object notation files.

Options

• None currently supported

TypeScript

Tslint

Uses the tslint tool to review TypeScript files. You need to include a tslint.json configuration file in your project or use the config option to provide a path to a config file.

Options

• config The config file you want tslint to use.
• project The tsc config file you want tslint to use.

CSS:

CSSLint

Uses the csslint npm module to check css files. Before you can use this linter you'll need to install nodejs and the csslint npm package:

cd path/to/lintreview
npm install

Both warnings and errors will be turned into code review comments. If you don't want code review comments for specific rules, you should ignore them.

Options

• ignore A comma separated list of rule ids to ignore.

SASS / SCSS

scss-lint

Uses the sass-lint npm module to check scss and sass files.

Options

• ignore A comma separated list of files to ignore.
• config Project relative path to the sass-lint config file you want applied.

Stylelint

Uses stylelint to check, scss, sass, less or css files.

Options

• config Provide a path to your stylelint configuration file. If none is provided then stylelint's default config file lookup behavior will be used, and falling back to the stylelint-config-recommended will be used.

Ruby:

RuboCop

Uses the rubocop gem to check ruby files. You'll need to install it to use it:

gem install rubocop

Options

• display_cop_names Set to true to pass display cop names in offense messages.

.rubocop.yml files will be respected, as described here.

Puppet:

puppet-lint

Uses the puppet-lint gem to check puppet manifests against the puppetlabs style guide.

You'll need to install it to use it:

gem install puppet-lint

Options

• config Provide a path to a puppet-lint config file.
• fixer_ignore A comma separated list of linter checks to ignore when running the fixer.

.puppet-lint.rc files will also be respected, to allow each project to disable checks. A list of checks can be found by running "puppet-lint --help"

Chef:

Foodcritic

Uses the Foodcritic gem to check Chef files. You'll need to install Foodcritic:

gem install foodcritic

Options

• path If your cookbooks aren't stored in the root, use this to set the path that foodcritic runs against. Example: path = cookbooks

Yet Another Markup Language:

yamllint

Uses the yamllint module to check yaml and yml files.

Options

• config Provide a path to the yaml config file for yamlhint.

Shell

Shellcheck

Uses shellcheck to lint shell scripts.

Options

• shell Select which shell to use. Options are: bash, sh, ksh or zsh. Default: sh
• exclude String of checks to ignore. Example: SC2154,SC2069

Ansible

Ansible-lint

Uses ansible-lint to lint Ansible plays.

Options

• ignore Set which ansible-lint error codes you wish to ignore.

Go lang

Golint

Uses go-lint to lint go code.

Options

• min_confidence Set the confidence level of a problem before it is reported.
• ignore A list of regular expressions, that allow you to ignore warnings.

Kotlin

Ktlint

Uses ktlint to lint kotlin code.

Options

• android Set to true to pass rule set in error messages.
• config Provide a path to the .editorconfig file for ktlint.
• experimental Set to true to add experimental rules.
• fixer Set to true to fix any deviations from the code style
• ruleset Provide "ruleset" a path to JAR containing one or more Rules gathered together in a RuleSet.

Lua

Luacheck

Uses luacheck to lint Lua code.

Options

• config Provide a path to the config file for luacheck.

Elixir

Credo

Uses credo to lint Elixir code. Runs mix credo list to get results by files.

Options

• checks Only include checks that match the given strings
• config-name Use the given config instead of "default"
• ignore-checks Ignore checks that match the given strings
• all Show all issues
• all-priorities Show all issues including low priority ones
• strict Show all issues and all priorities

Markdown

Remarklint

Uses remark-lint to lint markdown files. Will use the .remarkrc files in your project. The remark-preset-lint-recommended package is installed, other presets require customizing the nodejs docker image.

Options

• No options at this time.

lint-review's Issues

Add shellcheck

https://github.com/koalaman/shellcheck

Should be easy to add, it supports checkstyle formatting

js files are being linted with both eslint and jshint

We recently implemented eslint into some of our projects and we want these projects to be linted only with eslint. .lintrc files were updated there, jshint was replaced with eslint, however, jshint still reviews those projects and posts reviews along with eslint, is there some misconfiguration on our side or we have to get rid from jshint entirely?
Any reply is much appreciated.

Refactor code to make life easier

The direct use of the github client in the Review and Processor makes a number of testing scenarios much harder than they need to be. Instead it might make life simpler if there was an application service that wrapped github and provided an abstracted API that was simpler to mock. The Review and Processor classes could use this instead.

Public repository in Github enterprise cannot clone.

Inputing user name and Oath token or password is necessary to clone public repository in GhE.
But the process of cloning public repository in GhE is not in git.py#L75 but in git.py#L75.

So that, the worker process stops in the process of cloning.
The process of cloning public repository in GhE must be like private_clone.

Support for multiple language versions

I'm running lint-review for several projects tracked on a GitHub Enterprise instance, and it's working great for us, but one of the engineering teams I support maintains mixed Python 2.7 and 3.x apps.

I'm comfortable with hacking something up for us to use internally, but wanted to see if you have a preference for implementation in case you'd consider a PR; if nothing else, contributing my changes upstream means less work maintaining my fork.

In terms of implementation, the choices seem to be:

1. Add config option for command path

This could be done one of two ways -- either a configurable command name (absolute path, prefixed/suffixed) or an option to push directories onto $PATH

This change would be relatively straightforward to implement with a config like:

[tools]
linters = flake8

[tool_flake8]
select = E702
executable = flake8-3.5

2. Add more tools
   This doesn't seem like a very good approach as it would entail adding a dozen tools. Even if they were sub-classes of the "main" tool of a given type, all the added code (including tests) would be a chore to maintain

3. Some other clever thing
   My idea here would be something along the lines of adding a new config element amounting to a mapping of interpreter name to a command to "activate" it when the Processor class is initialized.

INTERPRETERS = {
    'python-3.5': '/opt/python/3.5/bin/activate',
    'ruby-2.2': "export PATH='/opt/ruby/2.2/bin:$PATH'", }

This would probably end up being buggy and unpredictable, and in fact, seems like the worst solution now that I write it out.

---

If this is something you'd consider accepting a PR for, I'd think option 1 would be the way to go, unless you have another suggestion

Could not publish job to celery. Make sure its running

I use docker implementation.

INFO 2017-06-22 18:51:27,384 lintreview.web web:67 Scheduling pull request for user/repo 1211
ERROR 2017-06-22 18:51:27,385 lintreview.web web:70 Could not publish job to celery. Make sure its running.

in the broker logs I continuously have:
=INFO REPORT==== 22-Jun-2017::18:54:16 ===
accepting AMQP connection <0.2437.0> (172.19.0.3:43665 -> 172.19.0.2:5672)

=INFO REPORT==== 22-Jun-2017::18:54:16 ===
connection <0.2437.0> (172.19.0.3:43665 -> 172.19.0.2:5672): user 'guest' authenticated and granted access to vhost '/'

=WARNING REPORT==== 22-Jun-2017::18:54:16 ===
closing AMQP connection <0.2437.0> (172.19.0.3:43665 -> 172.19.0.2:5672, vhost: '/', user: 'guest'):
client unexpectedly closed TCP connection

Could you help me to resolve it?

Add the ability to do apply different flake8 rules to different parts of a code base

I'm not sure if this is a good idea or not, but we're currently finding that we're linting different parts of code to different standards. It would be nice to have lint-review test different parts of our code with different rulesets.

Currently we just ignore the warnings from lint-review, or just disable those tests across the entire repo.

For example we're not as strict on docstrings in our tests because unittest sometimes uses the docstring as the test description (https://docs.python.org/2/library/unittest.html#unittest.TestCase.shortDescription).

Add JSCS

We should add JSCS to lintreview: http://jscs.info

(this is sort of a reminder for me to do it some day)

Is Celery Running?

I am using docker compose to run the lint review bot but I am getting this error below

INFO 2017-06-04 17:07:02,973 github3 models:167 JSON was not returned
DEBUG 2017-06-04 17:07:02,973 lintreview.web web:60 lintrc file contents ''
INFO 2017-06-04 17:07:02,973 lintreview.web web:67 Scheduling pull request for name/repo-link
ERROR 2017-06-04 17:07:02,974 lintreview.web web:70 Could not publish job to celery. Make sure its running.
"172.22.0.1 - - [04/Jun/2017:17:07:02] "POST /review/start HTTP/1.1" 500 - "-" "GitHub-Hookshot/a24207a"
INFO 2017-06-04 17:07:02,976 gunicorn.access glogging:309 "172.22.0.1 - - [04/Jun/2017:17:07:02] "POST /review/start HTTP/1.1" 500 - "-" "GitHub-Hookshot/a24206a"

Was wondering if the current docker setup is working or if I am missing something.

Huge diffs fail with a stack trace

When lint review tries to process a huge diff it fails in a bad way.

[2013-06-06 12:54:19,286: ERROR/MainProcess] 'File' object has no attribute 'patch'
Traceback (most recent call last):
  File "lintreview/tasks.py", line 43, in process_pull_request
    processor.load_changes()
  File "lintreview/processor.py", line 26, in load_changes
    self._changes = DiffCollection(pull_request_patches)
  File "lintreview/diff.py", line 14, in __init__
    self._add(change)
  File "lintreview/diff.py", line 17, in _add
    change = Diff(content)
  File "lintreview/diff.py", line 78, in __init__
    self._parse_diff(data.patch)
AttributeError: 'File' object has no attribute 'patch'

It should instead post a comment that the diff was too big and could not be reviewed.

If there are more than X comments a summary should be posted instead

Add a configuration option to stop posting line comments if there are more than summary_threshold comments in any review. Instead of spamming a pull request with hundreds of line comments it would be nicer if one large comment was posted with each violation in a bulleted list.

Gitlab Support

It would be great if this could also work with Gitlab!

Is there a cli version of lint-review?

I don't understand this README.

It is not written how to change image of docker.
and...
docker description are scattered far and wide.

Add json linting

It would be really nice if lint-review could lint .json files. Possibly with https://www.npmjs.com/package/jsonlint

JSON linter not commenting on the correct line

$ cat test.json
{
  "one": "value",
  "two": 2,
  "three": "three",
}
$ jsonlint test.json
test.json:5:0: Warning: Strict JSON does not allow a final comma in an object (dictionary) literal
   |  At line 5, column 0, offset 52
   |  Object started at line 1, column 0, offset 0 (AT-START)
test.json: ok, with warnings

If "three": "three" was added in the PR, lintreview wouldn't comment on this error, as it reports the error 1 line down.

I'm not sure what the solution so this problem is.

Add ability to ignore some branches

We have a special branch for releases, any code that gets in here is already linted. It would be nice to have an option to ignore some branches in lint-review.

Add support for puppet-lint

By using puppet-lint lint-review can automate code style checks for puppet modules.

flake8 exclude doesn't work as expected for directories

With flake8 alone, if I try to exclude my documentation directory as follows, it works:

flake8 --exclude=doc/ .

However, Stickler-CI includes each file explicitly in the command line invocation, e.g.,

flake8 --exclude=doc/ doc/examples/_code/weather_data_setup.py

With this version, the explicitly listed file is no longer included. Instead, I spent a while being frustrated (thinking I was misusing flake8) and eventually found Stickler's files.ignore option.

It might be worth noting this in the docs somewhere. There appears to already be a bug report upstream for flake8: https://gitlab.com/pycqa/flake8/issues/392

Lint riview not working

Suddenly I started getting the below error. Nothing changed at my end.

SSLError: bad handshake: Error([('SSL routines', 'SSL23_GET_SERVER_HELLO', 'tlsv1 alert protocol version')],)
INFO 2018-03-08 13:05:50,010 gunicorn.access glogging:309 "192.30.252.39 - - [08/Mar/2018:13:05:50] "POST /review/start HTTP/1.1" 500 - "-" "-"

What do I to resolve this?

Support multiple projects in a single repository

Usually, for web applications with NODEJS, developers create a folder structure like this:

app\ ===> which is the root folder for NODEJS source
app\public\ ===> which is the root folder for front-end source
and JSHINT config file for each of these projects is different.

It would be great to be able to set different tools and different configs for each of these projects.

One solution could be let the user put ".lintrc" file in any folder, and that folder will be the root of a project with specific config.

Use pull request build status instead of comment/label

When a check passes/fails update the build status using the repo status API. This is less noisy alternative to using comments, or issue labels.

The default mode going forward would be to use the status API, in addition the label behavior can optionally enabled.

Add dependency lists for various tools

Relying on system installed tools is getting pretty annoying as the number of tools that lint-review support grows. To curb the reliance on system libraries, it would be best to use each language's packaging system to install non-global libraries.

• For node use npm
• For PHP use composer
• For ruby use bundler but not RVM/rbenv.
• Python is already taken care of.

I think it would also be useful to have a simpler install workflow that is as simple as make install.

Linting fails if there's no reviewable changes

lint-review/lintreview/review.py

Lines 231 to 243 in 98488db

	def publish_empty_comment(self):
	self.remove_ok_label()
	body = ('Could not review pull request. '
	'It may be too large, or contain no reviewable changes.')
	comment = IssueComment(body)
	comment.publish(self._repo, self._pr)
	if self.config.get('PULLREQUEST_STATUS', True):
	self._repo.create_status(
	self._pr.head,
	'error',
	body
	)

That sends an error status to the github api. I made a valid PR that just deleted code, but the bot made my build fail.

Would it make sense to switch this to "success" ? I'm not 100% sure if that's right either.

SSL error

It looks like github have changed something (SSL because of Poodle?) and pygithub3 is throwing this SSL error:

WARNING 2014-10-15 07:21:06,526 lintreview.web web:57 [Errno 8] _ssl.c:480: EOF occurred in violation of protocol

It looks likepygithub3 is using an ancient version of requests which is part of the problem. There seems to be a few forks that fix this, here is one of them: https://github.com/intellisense/python-github3

When line numbers change errors are reported incorrectly.

When a diff introduces a big shift in line numbers the reported errors are put on the wrong lines.

For example if a pull request remove 30 lines and adds 10 and the added lines have a lint error in the first line comment will be misplaced.

Status updates need to check for permissions

This stopped the comment from happening, we should either fail more gracefully or raise an appropriate error. Not sure when the right time to raise the error is though.

A bot with a read-only token can comment and do the rest of the things it seems, but not update the status: https://developer.github.com/v3/repos/statuses/#create-a-status

[2016-07-25 13:36:49,304: INFO/Worker-8] Building a url from ('https://api.github.com/repos/Superbalist/XXXXXX', 'statuses', '08330c85fd6ee16508f2d00f42b1406e519f4916')
[2016-07-25 13:36:49,419: INFO/Worker-8] JSON was not returned

Add ESLint HTML plugin to package.json

Requesting inclusion of the eslint html plugin: https://www.npmjs.com/package/eslint-plugin-html

Failing PHP CS Fixer Tool Tests

Hi,

Trying to boot lint-review using docker-compose and I'm seeing the following:

======================================================================
FAIL: test_process_files__one_file_fail (tests.tools.test_phpcs.Testphpcs)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/code/tests/tools/test_phpcs.py", line 45, in test_process_files__one_file_fail
    eq_(12, len(problems))
AssertionError: 12 != 3
-------------------- >> begin captured logging << --------------------
lintreview.tools.phpcs: DEBUG: Processing ['tests/fixtures/phpcs/has_errors.php'] files with phpcs
lintreview.tools: DEBUG: Running phpcs --report=checkstyle --standard=PSR2 --extensions=php tests/fixtures/phpcs/has_errors.php
lintreview.review: DEBUG: Adding error '<class 'lintreview.review.Comment'>(filename=tests/fixtures/phpcs/has_errors.php, line=14, position=14, body=Opening brace should be on a new line)'
lintreview.review: DEBUG: Adding error '<class 'lintreview.review.Comment'>(filename=tests/fixtures/phpcs/has_errors.php, line=15, position=15, body=Spaces must be used to indent lines; tabs are not allowed)'
lintreview.review: DEBUG: Adding error '<class 'lintreview.review.Comment'>(filename=tests/fixtures/phpcs/has_errors.php, line=16, position=16, body=Spaces must be used to indent lines; tabs are not allowed)'
--------------------- >> end captured logging << ---------------------

======================================================================
FAIL: test_process_files_two_files (tests.tools.test_phpcs.Testphpcs)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/code/tests/tools/test_phpcs.py", line 65, in test_process_files_two_files
    eq_(12, len(problems))
AssertionError: 12 != 3
-------------------- >> begin captured logging << --------------------
lintreview.tools.phpcs: DEBUG: Processing ['tests/fixtures/phpcs/no_errors.php', 'tests/fixtures/phpcs/has_errors.php'] files with phpcs
lintreview.tools: DEBUG: Running phpcs --report=checkstyle --standard=PSR2 --extensions=php tests/fixtures/phpcs/no_errors.php tests/fixtures/phpcs/has_errors.php
lintreview.review: DEBUG: Adding error '<class 'lintreview.review.Comment'>(filename=tests/fixtures/phpcs/has_errors.php, line=14, position=14, body=Opening brace should be on a new line)'
lintreview.review: DEBUG: Adding error '<class 'lintreview.review.Comment'>(filename=tests/fixtures/phpcs/has_errors.php, line=15, position=15, body=Spaces must be used to indent lines; tabs are not allowed)'
lintreview.review: DEBUG: Adding error '<class 'lintreview.review.Comment'>(filename=tests/fixtures/phpcs/has_errors.php, line=16, position=16, body=Spaces must be used to indent lines; tabs are not allowed)'
--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------
Ran 136 tests in 42.757s

FAILED (SKIP=9, failures=2)
Service 'image' failed to build: The command '/bin/sh -c nosetests -v' returned a non-zero code: 1

It looks as though it's expecting PHP CS to find 12 errors in the file, but it's only getting 3. Running the tool manually (using PHP_CodeSniffer version 2.3.3) locally returns 3 as well:

phpcs --report=checkstyle --standard=PSR2 tests/fixtures/phpcs/has_errors.php
<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="2.3.3">
<file name="/Users/ollie/Code/lint-review/tests/fixtures/phpcs/has_errors.php">
 <error line="14" column="36" severity="error" message="Opening brace should be on a new line" source="Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine"/>
 <error line="15" column="1" severity="error" message="Spaces must be used to indent lines; tabs are not allowed" source="Generic.WhiteSpace.DisallowTabIndent.TabsUsed"/>
 <error line="16" column="1" severity="error" message="Spaces must be used to indent lines; tabs are not allowed" source="Generic.WhiteSpace.DisallowTabIndent.TabsUsed"/>
</file>
</checkstyle>

The composer.lock file is tied to PHPCS 2.2.0 (squizlabs/PHP_CodeSniffer@b301c98) but it looks like PEAR installs 2.3.3:

Step 6 : RUN pear install PHP_CodeSniffer
 ---> Running in d8adf72d1c51
downloading PHP_CodeSniffer-2.3.3.tgz ...
Starting to download PHP_CodeSniffer-2.3.3.tgz (470,589 bytes)
...............................................................................................done: 470,589 bytes
install ok: channel://pear.php.net/PHP_CodeSniffer-2.3.3

Presumably something's changed between 2.2.0 and 2.3.3 that means less errors are being reported?

Regardless, this causes the boot to fail and makes it unusable without a lot of faff.

Ta

Ollie

Report linting of an entire project via the github status API

It would be nice if the result of a linting of the entire project is reported to the github status api (https://github.com/blog/1227-commit-status-api).
Then there should be an option to allow this call to fail if there are more then n linting issues

pep8-naming

We would like to install pep8-naming, but it requires flake8 2.0 or higher. Is there a reason why lint-review uses 1.7.0 or can this be bumped up?

Version bump

Because #57 and #55 and #53 have all landed, what about bumping the version?

flake8 --snippet

Add support for https://pypi.python.org/pypi/flake8-snippets/0.1

Suddenly stopped working

getting two errors when I check webhook settings on GitHub

Error code 204:
==> lintreview.access.log <==
"192.30.252.37 - - [28/Feb/2018:11:44:48] "POST /review/start HTTP/1.1" 204 - "-" "GitHub-Hookshot/7a06a51"

And Error code 500

INFO 2018-02-28 11:40:33,014 requests.packages.urllib3.connectionpool connectionpool:788 Starting new HTTPS connection (1): api.github.com
2018-02-28 11:40:33 [27774] [ERROR] Error handling request
Traceback (most recent call last):
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/gunicorn/workers/sync.py", line 125, in handle_request
    respiter = self.wsgi(environ, resp.start_response)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/ubuntu/lint-review/lintreview/web.py", line 57, in start_review
    gh = get_repository(app.config, head_user, head_repo)
  File "/home/ubuntu/lint-review/lintreview/github.py", line 26, in get_repository
    return gh.repository(owner=user, repository=repo)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/github3/github.py", line 1138, in repository
    json = self._json(self._get(url), 200)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/github3/models.py", line 185, in _get
    return self.session.get(url, **kwargs)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/requests/sessions.py", line 487, in get
    return self.request('GET', url, **kwargs)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/github3/session.py", line 88, in request
    response = super(GitHubSession, self).request(*args, **kwargs)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/requests/sessions.py", line 475, in request
    resp = self.send(prep, **send_kwargs)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/requests/sessions.py", line 585, in send
    r = adapter.send(request, **kwargs)
  File "/home/ubuntu/lint-review/env/local/lib/python2.7/site-packages/requests/adapters.py", line 477, in send
    raise SSLError(e, request=request)
SSLError: bad handshake: Error([('SSL routines', 'SSL23_GET_SERVER_HELLO', 'tlsv1 alert protocol version')],)```

Linting fails when previous lint job hasn't finished

I'm noticing an issue that can cause linting to fail for projects where a linter run is in progress for a given PR and more commits are added to the PR.

This happens for me most regularly when using fixers on a copy of a repo with a lot of small files. It looks like the working directory isn't quite empty by the time the next webhook is delivered.

Linting and fixing run OK here:

worker_1  | [2018-04-23 18:06:06,361: INFO/Worker-1] JSON was returned
worker_1  | [2018-04-23 18:06:06,362: INFO/Worker-1] Building a url from ('https://ghe.contoso.com/api/v3/repos/alzabo/infrastructure-puppet/pulls/2', 'commits')
worker_1  | [2018-04-23 18:06:06,363: INFO/Worker-1] Missed the cache building the url
worker_1  | [2018-04-23 18:06:06,363: INFO/Worker-1] Running fixers on 4 files
worker_1  | [2018-04-23 18:06:06,364: INFO/Worker-1] Running fixer rubocop on 1 files
worker_1  | [2018-04-23 18:06:06,364: INFO/Worker-1] Running ruby2 container
worker_1  | [2018-04-23 18:06:07,919: INFO/Worker-1] Running fixer puppet-lint on 1 files
worker_1  | [2018-04-23 18:06:07,920: INFO/Worker-1] Running ruby2 container
worker_1  | [2018-04-23 18:06:08,587: INFO/Worker-1] Using commit workflow to apply fixer changes
worker_1  | [2018-04-23 18:06:11,941: INFO/Worker-1] Running lint tools on 4 files
worker_1  | [2018-04-23 18:06:11,942: INFO/Worker-1] Running rubocop on 1 files
worker_1  | [2018-04-23 18:06:11,942: INFO/Worker-1] Running ruby2 container
worker_1  | [2018-04-23 18:06:13,083: INFO/Worker-1] Running puppet-lint on 1 files
worker_1  | [2018-04-23 18:06:13,084: INFO/Worker-1] Running ruby2 container
worker_1  | [2018-04-23 18:06:13,703: INFO/Worker-1] Building a url from ('https://ghe.contoso.com/api/v3/repos/alzabo/infrastructure-puppet/pulls/2', 'comments')
worker_1  | [2018-04-23 18:06:13,704: INFO/Worker-1] Missed the cache building the url
worker_1  | [2018-04-23 18:06:13,777: INFO/MainProcess] Received task: lintreview.tasks.process_pull_request[f4227a74-4cfc-4ed1-9b9b-ed2202434792]
worker_1  | [2018-04-23 18:06:13,780: INFO/Worker-2] Starting to process lint for 
worker_1  | [2018-04-23 18:06:13,781: INFO/Worker-2] Loading pull request data from github. user=alzabo repo=infrastructure-puppet number=2
worker_1  | [2018-04-23 18:06:13,782: INFO/Worker-2] Building a url from ('https://ghe.contoso.com/api/v3', 'repos', 'alzabo', 'infrastructure-puppet')
worker_1  | [2018-04-23 18:06:13,782: INFO/Worker-2] Missed the cache building the url
worker_1  | [2018-04-23 18:06:13,898: INFO/Worker-1] Attempting to get JSON information from a Response with status code 200 expecting 200
worker_1  | [2018-04-23 18:06:13,900: INFO/Worker-1] JSON was returned
worker_1  | [2018-04-23 18:06:13,905: INFO/Worker-1] Publishing review of 0 new comments for alzabo/infrastructure-puppet/pull/2
worker_1  | [2018-04-23 18:06:14,199: INFO/Worker-2] Attempting to get JSON information from a Response with status code 200 expecting 200
worker_1  | [2018-04-23 18:06:14,200: INFO/Worker-2] JSON was returned
worker_1  | [2018-04-23 18:06:14,204: INFO/Worker-2] Building a url from ('https://ghe.contoso.com/api/v3/repos/alzabo/infrastructure-puppet', 'pulls', '2')
worker_1  | [2018-04-23 18:06:14,204: INFO/Worker-2] Missed the cache building the url
worker_1  | [2018-04-23 18:06:14,226: INFO/Worker-1] Attempting to get JSON information from a Response with status code 201 expecting 201
worker_1  | [2018-04-23 18:06:14,226: INFO/Worker-1] JSON was returned
worker_1  | [2018-04-23 18:06:14,227: INFO/Worker-1] Building a url from ('https://ghe.contoso.com/api/v3/repos/alzabo/infrastructure-puppet', 'statuses', '69e44942da37f5e741661474df3b516246eb0096')
worker_1  | [2018-04-23 18:06:14,359: INFO/Worker-2] Attempting to get JSON information from a Response with status code 200 expecting 200
worker_1  | [2018-04-23 18:06:14,360: INFO/Worker-2] JSON was returned
worker_1  | [2018-04-23 18:06:14,363: INFO/Worker-2] Building a url from ('https://ghe.contoso.com/api/v3/repos/alzabo/infrastructure-puppet', 'statuses', 'dfd8bcdabeadc24aa056cc5d60b6dbcc5b12cedf')
worker_1  | [2018-04-23 18:06:14,364: INFO/Worker-2] Missed the cache building the url
worker_1  | [2018-04-23 18:06:14,367: INFO/Worker-1] Attempting to get JSON information from a Response with status code 201 expecting 201
worker_1  | [2018-04-23 18:06:14,367: INFO/Worker-1] JSON was returned
worker_1  | [2018-04-23 18:06:14,369: INFO/Worker-1] Completed lint processing for alzabo/infrastructure-puppet/2
alzabo/infrastructure-puppet/2

The next webhook starts to be evaluated here:

worker_1  | [2018-04-23 18:06:14,483: INFO/Worker-2] Attempting to get JSON information from a Response with status code 201 expecting 201
worker_1  | [2018-04-23 18:06:14,484: INFO/Worker-2] JSON was returned

Cloning fails:

worker_1  | [2018-04-23 18:06:14,485: INFO/Worker-2] Cloning repository 'https://ghe.contoso.com/alzabo/infrastructure-puppet.git' into '/tmp/workspace/alzabo/infrastructure-puppet/2'
worker_1  | [2018-04-23 18:06:14,494: ERROR/Worker-2] STDERR output: fatal: destination path '/tmp/workspace/alzabo/infrastructure-puppet/2' already exists and is not an empty directory.
worker_1  |
worker_1  | [2018-04-23 18:06:14,495: ERROR/Worker-2] Unable to clone repository into '/tmp/workspace/alzabo/infrastructure-puppet/2'
worker_1  | [2018-04-23 18:06:14,495: ERROR/Worker-2] Unable to clone repository into '/tmp/workspace/alzabo/infrastructure-puppet/2'
worker_1  | Traceback (most recent call last):
worker_1  |   File "/code/lintreview/tasks.py", line 51, in process_pull_request
worker_1  |     git.clone_or_update(config, clone_url, target_path, pr_head)
worker_1  |   File "/code/lintreview/git.py", line 76, in clone_or_update
worker_1  |     authenticated_clone(config, url, path)
worker_1  |   File "/code/lintreview/git.py", line 45, in authenticated_clone
worker_1  |     clone(url, path)
worker_1  |   File "/code/lintreview/git.py", line 17, in wrapper
worker_1  |     return func(*args, **kwargs)
worker_1  |   File "/code/lintreview/git.py", line 55, in clone
worker_1  |     raise IOError(u"Unable to clone repository into '{}'".format(path))
worker_1  | IOError: Unable to clone repository into '/tmp/workspace/alzabo/infrastructure-puppet/2'
worker_1  | [2018-04-23 18:06:14,502: INFO/MainProcess] Task lintreview.tasks.process_pull_request[f4227a74-4cfc-4ed1-9b9b-ed2202434792] succeeded in 0.722406757995s: None

The workspace is cleaned up:

worker_1  | [2018-04-23 18:06:14,862: INFO/Worker-1] Cleaned up pull request alzabo/infrastructure-puppet/2
worker_1  | [2018-04-23 18:06:14,866: INFO/MainProcess] Task lintreview.tasks.process_pull_request[4a1e30c7-3cb7-4a62-a024-75a4cec11551] succeeded in 33.171103843s: None
[root@lint-review2 lint-review]# ls -la /tmp/workspace/alzabo/infrastructure-puppet/2
ls: cannot access /tmp/workspace/alzabo/infrastructure-puppet/2: No such file or directory
[root@lint-review2 lint-review]# ls -la /tmp/workspace/alzabo/infrastructure-puppet/
total 8
drwxr-xr-x 2 root root 4096 Apr 23 18:06 .
drwxr-xr-x 3 root root 4096 Apr 23 18:05 ..
[root@lint-review2 lint-review]#

Installing GitHub hooks via Docker after settings configuration doesn't work

Following the instructions for setting up webhooks I ran into a problem.

After configuring GITHUB_OAUTH_TOKEN token and other fields in settings.py in Lint Review Configuration I tried installing webhooks as per Installing GitHub hooks but I got a Bad credentials error.

Checking the settings.py in the web container, it seems untouched. The GITHUB_OAUTH_TOKEN and other fields were still in their respective default values.

Restarting the container with docker-compose restart web still doesn't reflect the changes.

Please advise, thanks!

Internal Server error occurs.

I'm going to use this lint-review with GitHub Enterprise.

I creat AMI by packer and start instance in AWS.
And I set following parameters in /etc/environment in AWS EC2 instance.

PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games"
LINTREVIEW_GUNICORN_BIND="0.0.0.0:5000"
LINTREVIEW_SETTINGS="/home/ubuntu/settings.py"
LINTREVIEW_SERVER_NAME="hostname:5000"
GITHUB_URL="githubURL"
GITHUB_USERNAME="githubUserName"
GITHUB_PASSWORD="githubPassword"

Then, I register the repository , and X-GitHub-Event: ping event has succeeded.
But I got internal server error with X-GitHub-Event: pull_request event.

This following message is stacktrace.

INFO 2017-03-28 13:16:00,056 lintreview.web web:48 Received GitHub pull request notification for git://ghe.misosiru.io/hotpepper-gourmet/perolin.web.git 1175, (reopened) from: git://ghe.misosiru.io/hotpepper-gourmet/perolin.web.git
2017-03-28 13:16:00 [1290] [ERROR] Error handling request
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/gunicorn/workers/sync.py", line 125, in handle_request
    respiter = self.wsgi(environ, resp.start_response)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/usr/local/lib/python2.7/dist-packages/lintreview/web.py", line 57, in start_review
    gh = get_repository(app.config, head_user, head_repo)
  File "/usr/local/lib/python2.7/dist-packages/lintreview/github.py", line 26, in get_repository
    return gh.repository(owner=user, repository=repo)
AttributeError: 'NoneType' object has no attribute 'repository'
ERROR 2017-03-28 13:16:00,056 gunicorn.error glogging:260 Error handling request

Please tell me why internal server error occur and show me the appropriate settings.

Support for BitBucket

Hi all,

do you have any plans to support Bitbucket?

Thanks

shellcheck on stickler-ci doesn't understand the source directive

To reproduce:

#! /bin/bash
# shellcheck source=scripts/inc/librelease.sh disable=SC1091
source "${SCRIPT_HOME}/inc/librelease.sh"

Live example:
CommandPost/CommandPost#929 (review)

When pasted in shellcheck.net no issues are found.

Perhaps it is running an older build of shellcheck?

Logging for Commands run should be copy and pasteable

It would be nice if the logging for commands run was copy and pasteable. This would make debugging simpler.

Add support for github oauth tokens

In addition to username + password it would be ideal if lint-review supported the use of oauth tokens when talking to github. This would remove the need for additional user accounts or username/password combo's in the configuration file.

Support for Swift

Do you have any plans to support for Swift such as SwiftLint?

Add support for ruby linters

Both rubocop and cane seem like good candidates for inclusion. More research may be required to find out what more people use, and how easy each is to get running.

Make CSS lint and JSHint use local NPM packages

Requiring systemwide npm packages is a bit silly since npm is great at installing local packages. Update csslint and jshint to check for local packages and fallback to system packages.

Commitcheck tool not flagging invalid commits

Something is not right with the commitcheck tool. It is not flagging intentionally incorrect commits. This is mostly a reminder for myself to fix the issue.

Option to delete checkout when done

It would be very beneficial to have an option to delete the checkedout repo after the review is finished.
Our repos are on the rather large size, and the disk is filling up very fast due to all the checkouts.
I still think it's beneficial to start clean each time, it would just be nice to be able to clean up afterwards.

Build GitHub Tags

It'd be awesome to have versioned Docker images because at the moment the only real version is latest/master.

Add support for ignore file list

Not all linting tools provide a way to exclude certain file patterns/paths from them. Add a generic way to exclude file directories/filepatterns from being checked for lint errors.

This should be defined in the .lintrc file. Possible systems to model the implementation are .gitignore, glob and regexp.

F401 not within diffs

I recently had a commit where I removed some code, which then ended up causing an F401(Unused import), which fell outside the diff.

It would be nice to still comment on the PR that the changes caused the unused import.

It would be complicated to determine that the unused import was caused by this diff, but I think it might be okay to assume that before the PR the files had no unused imports.

Summary comments aren't ordered anymore

We've recently upgraded from 0.3.0 to 0.7.0 (v0.3.0...v0.7.0) and I've noticed that the summary comments are no longer in an order.

I'm sure this isn't supposed to be the case, as OrderedDict was introduced: v0.3.0...v0.7.0#diff-e9646f2c584bc4d8db229c3a84f91cd7R259