DM-36911: switch to reusable yamllint workflow

[mwittgen]

No description provided.

[gpdf]

What's the status of this branch? Was it stalled by the felis-linter error? If you @mwittgen update it to the current main and re-run the CI I'll have a look at it. There are some known problems with felis-linter but the other CI tests should not fail, and I should be able to tell you whether it's OK to merge or not.

[mwittgen]

I don't remember, but felis-linter was not the hold up. The ticket branch is rebased.

[gpdf]

The Felis Linter errors are the expected ones, so based on that I have no objections to proceeding, but it looks like something is wrong with the lint action -- it's still not complete after 14 hours.

[mwittgen]

I don't have admin access to this repo. Seems to be the old lint action that got replaced by the new reusable work flow one.
It just needs to be removed as an expected action in the github repo setup.

[gpdf]

I don't have admin access to this repo. Seems to be the old lint action that got replaced by the new reusable work flow one. It just needs to be removed as an expected action in the github repo setup.

@ktlim Is this something you could fix?

[ktlim]

We want to have yamllint required. But I think it would also be good to have CI build required. Unfortunately, the name of that action is the same as the name of the Felis Linter ("build"), and the branch protections cannot distinguish between the two. I have disabled the requirement for both "build"s for now, but we should change the name of the Felis Linter action and then make CI build required. Of course, once the Felis Linter passes regularly, we should make that required also.

[gpdf]

@JeremyMcCormick Can you take a look at whether we can get this PR closed out, now that we're sorting out other logjams?

[JeremyMcCormick]

@gpdf I tested this branch on a personal fork of the repo, and the yamllint tool seemed to run fine in GHA. The workflow completed without issues.

I believe the ticket branch needs to be rebased with main, and it should be configured to run on ticket branches and releases. It is currently setup to run on main so it doesn't trigger until after a merge occurs.

[JeremyMcCormick]

We want to have yamllint required. But I think it would also be good to have CI build required. Unfortunately, the name of that action is the same as the name of the Felis Linter ("build"), and the branch protections cannot distinguish between the two. I have disabled the requirement for both "build"s for now, but we should change the name of the Felis Linter action and then make CI build required. Of course, once the Felis Linter passes regularly, we should make that required also.

It looks to me that the "build" job was changed to 'yamllint" in rubin_workflows so I don't think this is an issue any longer.

@ktlim can you confirm?

[gpdf]

There's something I don't quite understand that's going on. If you look at this screenshot:

there appear to be two "Lint YAML Files" actions which (if you look at both mouseovers) differ only in arising from "yamllint.yml" and "yamllint.yaml". Is that related to this change?

[JeremyMcCormick]

@gpdf

there appear to be two "Lint YAML Files" actions which (if you look at both mouseovers) differ only in arising from "yamllint.yml" and "yamllint.yaml". Is that related to this change?

That's probably just because the file "yamllint.yml" was renamed to "yamllint.yaml" on this branch, but it should resolve once the PR is merged into main.

[gpdf]

@mwittgen Can you rebase this so we can get a final review from @ktlim and merge it?

[mwittgen]

Rebased

[JeremyMcCormick]

I am still seeing the branch marked as out of date. Can I press "Update with rebase" to fix?

[mwittgen]

Rebased once more. There have been more commits to main since the last rebase yesterday.

[gpdf]

@ktlim @mwittgen For the record, it appears that we had to disable required checks in order to merge this, because the deleted-by-this-PR "yamllint.yml" workflow (internal name "lint") was being shown as hanging, even though it wasn't in the branch. So evidently it was triggering off the (unmodified) workflows on main.

Once this is merged that override will not be necessary any more.

[mwittgen]

Yes, the lint workflow will be replaced by yamllint

[gpdf]

Somebody with admin privileges to this repo needs to disable the lint workflow as required

Yes, @JeremyMcCormick has admin and has done this -- we just wanted you and KT to know.

[gpdf]

I think you can merge now, @mwittgen .

[mwittgen]

Thanks for clearing out this ticket.