django-crispy-forms / crispy-tailwind

A Tailwind template pack for django-crispy-forms
MIT License
329 stars 56 forks source link

Run Linters in Pipeline / Actions #158

Open GitRon opened 4 months ago

GitRon commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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:

Best
Ronny

carltongibson commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

Yeah, if you can simplify, let's have it! ๐Ÿ‘

GitRon commented 4 months ago

Great, so I'll sum up:

Thx @carltongibson for your input! โค๏ธ

carltongibson commented 4 months ago

Not sure I did much. ๐Ÿ˜…

Thanks for your energy @GitRon ๐ŸŽ

smithdc1 commented 4 months ago

Thanks for your energyย @GitRonย ๐ŸŽ

I completely agree ๐Ÿ’ฏ

Thank you!