search

legrego / homeassistant-elasticsearch

Publish Home-Assistant events to Elasticsearch
https://legrego.github.io/homeassistant-elasticsearch/
MIT License
145 stars 38 forks source link

Improve workflow (again) #120

Closed KTibow closed 3 years ago

KTibow commented 3 years ago

I made ha-blueprint recently. This merges your existing workflow with the one I made.

KTibow commented 3 years ago
  1. Sure! I'll go ahead and remove that.
  2. I'll add using a predefined flake8 rule list to ha-blueprint when I have time, putting this in my saved notifications. Right now it checks for syntax errors, then installs wemake-python-styleguide (very strict linting) and gives you feedback so you can nitpick your code when you have time.
  3. No difference, basically the normal way is GitHub Actions builds the docker container and runs it, I'm just reverse-engineering how they did that and doing that if Python files are detected.
legrego commented 3 years ago
  1. Sure! I'll go ahead and remove that.

Thanks!

  1. I'll add using a predefined flake8 rule list to ha-blueprint when I have time, putting this in my saved notifications. Right now it checks for syntax errors, then installs wemake-python-styleguide (very strict linting) and gives you feedback so you can nitpick your code when you have time.

I see. The previous action would actually stop the build if we had violations, and I don't want to lose that. It seems silly to run flake8 twice, but maybe we can add the original check back into this project's workflow?

  1. No difference, basically the normal way is GitHub Actions builds the docker container and runs it, I'm just reverse-engineering how they did that and doing that if Python files are detected.

Thanks for explaining. Just as an FYI, I'm seeing a warning in the action output when installing isort -- not sure if you've seen this or not, but we may require updates soon: https://github.com/legrego/homeassistant-elasticsearch/runs/1268926009#step:8:875

ERROR: After October 2020 you may experience errors when installing or updating packages. This is because pip will change the way that it resolves dependency conflicts. Successfully installed isort-5.6.4

We recommend you use --use-feature=2020-resolver to test your packages with the new resolver before it becomes the default.

KTibow commented 3 years ago
  1. What kind of violations? Right now it'll fail on E9,F63,F7,F82. What do you mean, the original check? Plain flake8 with no modifications?
  2. Don't worry about it, everyone gets it if they update pip.
legrego commented 3 years ago

What kind of violations? Right now it'll fail on E9,F63,F7,F82.

Ah, I missed this part when reading the action code the first time around.

What do you mean, the original check? Plain flake8 with no modifications?

Almost without modifications (so far). We're overriding the max-line-length right now:

https://github.com/legrego/homeassistant-elasticsearch/blob/ad56b5dc9828de7c413ccd93da966e5e51570121/.flake8#L1-L2

My preference is to have GitHub Actions use the same rules that my IDE uses, so that I can get proper feedback before pushing my changes. Having different rules between the repo's config (.flake8) and the GH Actions's config means that I can't rely on my IDE to show me all of the errors that could go wrong.

Perhaps ha-blueprint can ship with E9,F63,F7,F82 as the default set of rules to fail on, but use the settings configured in the repo if they're provided? That would give other custom components the benefit of your "preferred" rules, while still letting stubborn people like me define their own rule set. What do you think?

KTibow commented 3 years ago

Looks good to you now?

KTibow commented 3 years ago

BTW consider opting this repo in to hacktoberfest (just add the topic).