Feature/1412 progressive ref

[MarcinWieczorek]

#1412

This POC works and a testcase has been added. I'm waiting on suggestions regarding the final shape of the options. A backward compatibility issue was mentioned in the discussion (progressive: true in the config).

[ssbarnea]

@MarcinWieczorek I am quite interested about what is the correct way to retrieve the ref from within GHA runners because I do not see any practical use to have it configured inside config file or even as an argument from cli.

Still, I do understand the real need to be able to infer the correct value when running on CI/CD pipelines as current approach of looking at the last commit clearly breaks for any pull-request with more than one commit.

[MarcinWieczorek]

CI would have to pass the ref to the workflow. Github provides variables for this: https://docs.github.com/en/actions/learn-github-actions/contexts#github-context
It should be either github.head_ref or github.base_ref.
Action would have an input with proper default. Then it can be passed to cli here: https://github.com/ansible/ansible-lint-action/blob/main/action.yml#L33

Am I correct?

[ssbarnea]

@MarcinWieczorek I am trying to find a solution for this issue but it is not hard at all, mainly because your change would not fix it for anyone. I am even wondering if adding progressive mode was really a good idea in the first place.

Unless user knows to add the extra parameter which mentions the number of commits, it will still look for the last one. Computing the right value on all/most used CI/CD systems would be hard to hard to do,... and I realise almost nobody will bother to do it.

I am considering fully removing the progressive mode and replacing it with and ignore list similar with the one used by ansible-test, where you could dump the known failures and the linter will know to ignore the known ones, reporting only failures not listed inside that ignore file. That would be a different way to achieve the same "progressive" approach.

Now, once issue that I see with that is that usually a violations contains: filename, tag and often a line number. Still, that line number would change if someone adds/removes entries before that entry, making the ignore file drift (partially invalid).

[ssbarnea]

@MarcinWieczorek I had some long talks this week with other team mates and we decided to go in a little bit different direction and deprecated progressive mode, this bug being one of the reasons.

I already have a POC change for the new feature that should be a relatively decent replacement for the current progressive mode at #3004

Basically we will load ignores from a simple text file and when adopting the linter you can generate an ignore file, which allows you to adopt the linter in a progressive manner. By fixing ignored issues from that file and reducing its size in time.

Please let me know what you think about it. We did use a similar approach with other linting tools in the past and it worked fine for us.

[MarcinWieczorek]

I will try your solution when I got time. How is this going to be implemented in ansible-lint-action? Is the user expected so set-up his ignore file manually?

[ssbarnea]

@MarcinWieczorek Yes, you need to generate this file and commit it. The process is the same as for other linters such flake9, pylint,...