Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Add support to default config files #447

Merged

Conversation

gpiress
Copy link
Contributor

@gpiress gpiress commented May 23, 2017

This PR tries to load the tox.ini file for loading linting rules.

The order of precedence is:

  • Tries to load projectConfigFile if set
  • Tries to load tox.ini if it exists
  • Loads default configurations

The idea behind this is that if a project already has a tox.ini
file, it should try to use unless overriden. This works similar
to when flake8 is run in the command line.

Fixes #124.

@gpiress
Copy link
Contributor Author

gpiress commented May 23, 2017

I don't really get the error message, seems unrelated to the changes.

Local runs of npm run lint and npm run test work fine, is there
anything else that I should look out for?

@Arcanemagus
Copy link
Member

Arcanemagus commented May 23, 2017

Bizarre, the projectConfigFile setting already exists to handle this, however it looks like the defaults were left blank. If I remember correctly flake8 at least used to run it's own config search if you didn't specify a --config option, if that is no longer the case then the default for projectConfigFile should just be updated to the names of config files that flake8 will load on it's own instead of introducing a hidden search.

Note the CI failure is due to a change in Atom and is unrelated to this PR.

@gpiress
Copy link
Contributor Author

gpiress commented May 23, 2017

@Arcanemagus currently, if the projectConfigFile is not explicitly set in the package settings page, the package does not apply the rules specified in tox.ini.

I'll update the PR to default projectConfigFile to the files that flake8 uses when running it from the command line.

@gpiress gpiress force-pushed the gpiress/default-linter-settings branch 2 times, most recently from e6a5e28 to 6290d06 Compare May 24, 2017 15:08
@gpiress
Copy link
Contributor Author

gpiress commented May 24, 2017

@Arcanemagus So, I updated the PR with the suggested changes.

Now it checks if the projectConfigFile is set, if it isn't, it looks for the patterns that flake8 looks for (tox.ini, .flake8 and setup.cfg, according to the docs).

I tested it out using all three different default config files, it works for all of them.

If the user sets the projectConfigFile, in the package settings, it overrides every other rule and uses that one.

If no config file is set and no default config file is found, it uses the default rules.

@Arcanemagus
Copy link
Member

Sorry, I literally meant change the default value here, that way instead of having a hidden default users can see explicitely what is being searched for and add/remove from it as they desire.

@gpiress
Copy link
Contributor Author

gpiress commented May 24, 2017

Oh, okay :)

First time dealing with atom packages, didn't even think of looking there :)

Set the default value for the `projectConfigFile` option
to the patterns that flake8 uses when running from command line,
as stated http://flake8.pycqa.org/en/latest/user/configuration.html.
@gpiress gpiress force-pushed the gpiress/default-linter-settings branch from 6290d06 to e030573 Compare May 24, 2017 22:29
@gpiress
Copy link
Contributor Author

gpiress commented May 24, 2017

Okay, this should do the trick now :)

Tested it locally and worked out.

@Arcanemagus
Copy link
Member

Marked this as fixing #124, since it likely will.

@Arcanemagus Arcanemagus merged commit 73b3fbd into AtomLinter:master May 24, 2017
@Arcanemagus
Copy link
Member

Published in v2.3.0 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants