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

Run Linters in Pipeline / Actions #158

Open
GitRon opened this issue Feb 10, 2024 · 8 comments
Open

Run Linters in Pipeline / Actions #158

GitRon opened this issue Feb 10, 2024 · 8 comments

Comments

@GitRon
Copy link
Contributor

GitRon commented Feb 10, 2024

Hi @smithdc1 and @carltongibson

am I wrong or don't we validate the linting rules in our CI pipeline?

grafik

There is something in the per-pyhton-version setup but apparently, it's not being executed.

Futhermore, linting should / can be checked version-independly, right?

What's your take on this?

Best
Ronny

@carltongibson
Copy link
Contributor

carltongibson commented Feb 10, 2024

See here: https://github.com/django-crispy-forms/crispy-tailwind/actions/runs/7828342050/job/21358001447

Latest successful run, Python 3.12. Linters are executed.

That corresponds to the workflow file:


      if: ${{ matrix.python-version == '3.12' }}

For me, we only need to run these for a single Python version.

So LGTM, I'd say.

@GitRon
Copy link
Contributor Author

GitRon commented Feb 11, 2024

Hi @carltongibson!

Thanks for the explanation! That's a novel setup to me! I was discussing in another ticket that an HTML linter might come in handy, therefore I looked a the setup.

So a couple of thoughts/question here:

  • In my projects, I use pre-commit in the pipeline to avoid having duplicated setups. Might this make sense here as well?
  • I know about "boring tech" but ruff is just so much better and faster than black and flake8 that it feels like dark middle ages to work with them. Might we replace them?
  • I find it a little counter-intuitive to have X python-steps ignore linting and one is executing it. I'd suggest we create a separate step (is this the correct term?) for this, like we have for deployment.
  • Finally, last question, unrelated to linters: Why are the docs running in a different pipeline? Isn't that a little overkill for just one tiny step?

Best
Ronny

@carltongibson
Copy link
Contributor

My preference is to keep configs in the tox file. The workflows just then call out to that. I'm not a fan of putting everything via pre-commit, though I know others think differently.

I'd rather not shift to ruff. It's controlled by a VC funded private company, and (as nice as the folks might be) the bill for that will become due one day. I would rather use community tools.

I'm not quite sure what you have in mind for the other points... Like, maybe 😅 — I think the idea with the current setup is to avoid running the linter step redundantly for every Python version. (Once is enough there)

@GitRon
Copy link
Contributor Author

GitRon commented Feb 11, 2024

My preference is to keep configs in the tox file. The workflows just then call out to that. I'm not a fan of putting everything via pre-commit, though I know others think differently.

Good point. I would still do it differently but see your point. We keep it as it is.

I'd rather not shift to ruff. It's controlled by a VC funded private company, and (as nice as the folks might be) the bill for that will become due one day. I would rather use community tools.

That's true. Ok, we stick with it for now.

I'm not quite sure what you have in mind for the other points... Like, maybe 😅 — I think the idea with the current setup is to avoid running the linter step redundantly for every Python version. (Once is enough there)

Sorry 😅 Here's a screenie of "my" setup. The linting just runs intependently of the unittests and I don't need the if-condition, still it only runs once per pipeline.

grafik

@carltongibson
Copy link
Contributor

Yeah, if you can simplify, let's have it! 👍

@GitRon
Copy link
Contributor Author

GitRon commented Feb 11, 2024

Great, so I'll sum up:

Thx @carltongibson for your input! ❤️

@carltongibson
Copy link
Contributor

Not sure I did much. 😅

Thanks for your energy @GitRon 🎁

GitRon added a commit to GitRon/crispy-tailwind that referenced this issue Feb 11, 2024
GitRon added a commit to GitRon/crispy-tailwind that referenced this issue Feb 11, 2024
GitRon added a commit that referenced this issue Feb 11, 2024
* #158: Optimise CI linting

* #158: Added linting to "needs" section of deploy job
@smithdc1
Copy link
Member

Thanks for your energy @GitRon 🎁

I completely agree 💯

Thank you!

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

No branches or pull requests

3 participants