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

Script/lint: Changed default from 'all' to 'changed' #12660

Merged
merged 1 commit into from
Mar 1, 2018
Merged

Script/lint: Changed default from 'all' to 'changed' #12660

merged 1 commit into from
Mar 1, 2018

Conversation

cdce8p
Copy link
Member

@cdce8p cdce8p commented Feb 25, 2018

Description:

Changed the default behavior of script/lint. Instead of checking all files on execution, only the changed ones will be tested for lint errors. Testing all files is still possible with flag --all or the call itself: tox -e lint.

Response to: #12651

@cdce8p cdce8p requested a review from emlove February 25, 2018 14:57
@homeassistant homeassistant added small-pr PRs with less than 30 lines. cla-signed labels Feb 25, 2018
@emlove
Copy link
Contributor

emlove commented Feb 25, 2018

I'm 👍 on this, but we should get a few opinions.

We should make sure to get a docs PR open as well. At least the Testing your code page needs to be updated, but it's probably worth a quick grep as well in the repo.

Copy link
Member

@kellerza kellerza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

(originally added --changed 👍 )

@cdce8p
Copy link
Member Author

cdce8p commented Feb 25, 2018

Just read through the doc. It was already described to just check changed files. Maybe we could even remove the --all flag, since running tox -e lint would take nearly the same time which is also described in the first section.

@kellerza
Copy link
Member

https://home-assistant.io/developers/development_testing/ clearly shows the --cleared flag... but this will still work with this version, better to update the docs as well though.

@emlove
Copy link
Contributor

emlove commented Feb 25, 2018

I'd be fine just dropping --all as well myself. Most everywhere else we instruct devs to just use tox directly.

@cdce8p
Copy link
Member Author

cdce8p commented Feb 25, 2018

@kellerza I already opened a PR for it: home-assistant/home-assistant.io/pull/4753

@cdce8p cdce8p merged commit 17ba813 into home-assistant:dev Mar 1, 2018
@cdce8p cdce8p deleted the script-lint branch March 1, 2018 17:02
@balloob balloob mentioned this pull request Mar 9, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed small-pr PRs with less than 30 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants