Lint only changed SQL files on PRs

[max-ostapenko]

Added diff logic to SQL linting task.
It finished linting SQL faster than it takes to download super-linter image.

1. SQLfluff as PIP dependency will run on sql diffs (+ full scan on sqlfluff version update or a manual trigger)
2. SQLfluff in super-linter is off.
3. SQL linting related to sqlfluff v3.5.0

[tunetheweb]

I've got some questions:

It finished linting SQL faster than it takes to download super-linter image.

It took 46 seconds, rather than 1 minute 38 seconds. So yes faster but only a minute faster. Is it worth running something different for SQLFluff for that? What's the issue you're looking to solve here?

Also at the minute "Lint Code Base" will run anyway so we'll need to prevent that running otherwise there's not saving right? This is also a required action so we'd need to remove that and make it optional (not sure that's a big deal - we've already turned off Test Website since it didn't always run). We normally check them anyway so note sure we need the enforcement.

I'm also tried running a total lint:

• With this change it takes 7 mins 45 seconds
• Using super-linter it takes 7 mins 48 seconds

So again no real saving. Both also seemed to pick up new errors btw. We should look to fix thise.

[max-ostapenko]

I suggest using the latest, lightweight SQL linter - via PIP.
PIP and super-linter may temporarily differ in versions, and current errors come from new rules in v3.5.0.
We’ll be forced to fix these before dependabot’s PR can auto-merge, if we deduplicate.

BTW Both your runs use PIP, not super-linter.
A full super-linter run takes ~25 min. But to be fair, PIP is roughly 2× faster start to finish.

[tunetheweb]

I suggest using the latest, lightweight SQL linter - via PIP.

Yeah not adverse to that. If we can update this the other points above to not run both.

Since your diff code is relatively easy maybe we should aim tom remove superlinter altogether? Depends if we have Python or JS versions of all the linters.

BTW Both your runs use PIP, not super-linter.
A full super-linter run takes ~25 min. But to be fair, PIP is roughly 2× faster start to finish.

🤦‍♂️ Ooops. Silly.

PIP and super-linter may temporarily differ in versions, and current errors come from new rules in v3.5.0.
We’ll be forced to fix these before dependabot’s PR can auto-merge, if we deduplicate.

So in this case these are extra checks. So fixing (or skipping with a --noqa comment since they are old SQL and not worth worrying too much about) won't break the superlinter version. But yes I agree in theory that could happen — I can't remember if it has in the past—certainly not been a big problem as far as I remember.

[max-ostapenko]

I don't think we should drop super-linter - we use ~10 linters and it's easy to just run all of them for the src/ folder.
SQL is special - it has a dedicated contributors and it's own folder (we know when to trigger linting).

I've updated all the points now.

[tunetheweb]

LGTM with a few nits