Fix error whitelist

[garrettXCIV]

More information about what changes in Pylint caused the initial linter-pylint bug here.

Fixes #236

[garrettXCIV]

@Arcanemagus Hey, can you do me a favor and look this over?

[ddaanet]

This would be simpler and just as correct:
/^Using config file /,

[Arcanemagus]

You're technically right that since this list of RegEx is tested against every line of stderr either version would work (and yours is slightly better for performance). The merged version works when testing against the entire stderr if the code is refactored later to do that, and matches the behavior of the existing RegEx though.

[ddaanet]

I committed the simplification directly on master before your reply.

Speculative benefit in case we start matching the entire stderr does not seem worthwhile.

The behaviour of the existing regex is correct because we do match the entire line, but the new regex only matches the start of the line.

So I favored the simpler code. I'll revert my change if you care.

[ddaanet]

Except for the regex that could be simpler, I confirm this fix the problem. Actually, I came here because I wrote this exact same fix for my own use.

[ddaanet]

Oh, well! It seems I can merge the PR. Cool!

[dundee]

Please release new version, this is major fix. Thanks!

[ddaanet]

@Arcanemagus sorry, I did not have the time and energy to figure out the release process. Life is keeping me quite busy...

[dundee]

Could I help somehow?

[ddaanet]

@dundee Slack ate the chat history… I think the relevant issue is this one: AtomLinter/Meta#18

[Arcanemagus]

@AtomLinter/linter-pylint If there are no objections to the Conventional Commits format being required for all commits here, I can set up the automated release process described in AtomLinter/Meta#18, meaning that as soon as commits are merged to master that would trigger a release it will happen automatically. See that issue for a bit more details.

For now I'll push out a release manually 😉.

[Arcanemagus]

Published in v2.1.1. 🎉

[ddaanet]

@Arcanemagus I am a bit wary of requiring Convential Commits format for all commits.

This will certainly create change requests during code review, that will produce rebase and force push, and tooling generally does not cope will with rebase. For one thing, github cannot show when the change was made, and it tends to create confusing code reviews (when comments are made on changes that appear later in the timeline after rebase).

I like the general idea of Conventional Commits, but the suggested implementation, based on squash, force-push and rebase is terrible. I would be happy with requiring conventional commit format only on master mainline commits (that is, PR merge commits).

All that being said, I would take automatic releases with conventional commits on all commits today over manual commits forever.

[Arcanemagus]

the suggested implementation, based on squash, force-push and rebase is terrible.

I'm a bit confused where this is "suggested"? Part of adding this would be adding a pre-commit hook that lints the commit message, ensuring it follows the proper format. So the only people that would need to rebase because of the commit message format would be people that explicitly disable that check (or failed to run npm install which means they probably also didn't lint their code, or test it...).

For example over in atom-linter-pug I've just been doing merge commits.

[Arcanemagus]

Ah, just searched through that Conventional Commits page, I just thought it outlined the format not suggested a workflow. It seems they wrote that assuming that messages would only be verified when doing the final merge, but that's far too error prone for my liking so I went with an automated setup that verifies every message.

Half the reason for doing a enforced style is it forces people to think about their commits and break them into isolated parts instead of bundling many different unrelated parts into a single commit.

This definitely isn't something everyone will want, which is why I'm still looking into alternative ways of doing this and haven't started rolling it out to more repositories beyond the test one.

[ddaanet]

@Arcanemagus

Ah, just searched through that Conventional Commits page, I just thought it outlined the format not suggested a workflow. It seems they wrote that assuming that messages would only be verified when doing the final merge, but that's far too error prone for my liking so I went with an automated setup that verifies every message.

A pre-commit hook does make it more practical.

Half the reason for doing a enforced style is it forces people to think about their commits and break them into isolated parts instead of bundling many different unrelated parts into a single commit.

For code review AND integration, it makes a lot more sense to aim for semantic granularity at the PR level.

• In my experience, code review tends to produce a lot of "coding style" and "cleanup" commits that are irrelevant for changelog and versioning. If the tooling makes this awkward, people will amend and rebase more. And that will break github code reviews.
• To keep diffs small enough for code review, a PR should not contain more than one semantic unit worthy of a changelog entry.
• Tools cannot force people to work in a certain way. That is what code review is for.

Really, most of my issues against conventional commits on all commits come from git having no support for tracking history when rebasing. This is arguably a problem with git. But here we are…