Skip to content

Script/lint, Lazytox: Fix issue to ignore delete files#13051

Merged
balloob merged 2 commits intohome-assistant:devfrom
cdce8p:script-lint-lazytox-fix
Mar 10, 2018
Merged

Script/lint, Lazytox: Fix issue to ignore delete files#13051
balloob merged 2 commits intohome-assistant:devfrom
cdce8p:script-lint-lazytox-fix

Conversation

@cdce8p
Copy link
Copy Markdown
Member

@cdce8p cdce8p commented Mar 10, 2018

Description:

git diff now ignores deleted files for pylint.

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4877

CC: @kellerza

@OttoWinter
Copy link
Copy Markdown
Member

OttoWinter commented Mar 10, 2018

In general, I'm for this fix. But in the lazytox.py script I think we could do better than just ignore deleted files.

Lazytox is supposed to be "intelligent" and automatically run matching tests. For example, deleting homeassistant/components/mqtt/__init__.py would surely invalidate the tests/components/mqtt/test_init.py tests. I think we can do better than that, especially when we have all the awesomeness of python available.

I think a small check in the lint() method for each file would suffice.

# in async def lint(files):
files = [f for f in files if os.path.isfile(f)]

@kellerza
Copy link
Copy Markdown
Member

We already test if files exist for pytest and would be nice to do the same for linting, then it also covers the commandline files and uses those for guessing pytest as Otto mentioned

Copy link
Copy Markdown
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.

🎉

@kellerza
Copy link
Copy Markdown
Member

Would have implemented it slightly different. Where pyfiles is created, print a warning for each non existent file and exclude from pyfiles.
But this will work!

@kellerza
Copy link
Copy Markdown
Member

Scratch that. We need pyfiles in tact for pytest calcs :-)

@balloob balloob merged commit 7ea7fc8 into home-assistant:dev Mar 10, 2018
@cdce8p cdce8p deleted the script-lint-lazytox-fix branch March 10, 2018 17:13
@balloob balloob mentioned this pull request Mar 30, 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.

5 participants