Add tox -e lint to GHA

[erik-whiting]

Resolves #2369

[erik-whiting]

If you check the build, you can see that the step updated the test_roles.py file

[matentzn]

If you do it like this, the files will be formatted wrongly in version control.. you have to: run the Linter and push the updated file to the branch!

[matentzn]

Excellent!

[matentzn]

@erik-whiting I approve, is this ok to merge?

[erik-whiting]

@matentzn no I don't think so. notice that the checks are still "Waiting for status to be reported." Something about the changes here are causing the checks to hang and I need to figure out why before merging or this will happen on every PR

[matentzn]

QC cannot run in response to a commit generated by GHA I think.. So if a change to a branch was induced by a GHA, then no other actions are run to avoid infinite snowballing

[erik-whiting]

oh I see, let's merge then 🤞

[matentzn]

The problem is though that we will never know then if the PR is broken for other reasons... I think the solution is to combine both actions (qc and lint), first run the lint, then the qc pipeline (make sure though you "check out" again). But I am not sure how these cases are solved, you may need to reach out to your colleagues or chatgpt:

"I have a GHA that updates my branch with a linter, and a GHA that runs a validation for my code - what is the best practice to make sure that the branch is first updated, then validated?"

[erik-whiting]

The problem is though that we will never know then if the PR is broken for other reasons... I think the solution is to combine both actions (qc and lint), first run the lint, then the qc pipeline (make sure though you "check out" again). But I am not sure how these cases are solved, you may need to reach out to your colleagues or chatgpt:

"I have a GHA that updates my branch with a linter, and a GHA that runs a validation for my code - what is the best practice to make sure that the branch is first updated, then validated?"

@matentzn See the screenshot below

This is from a PR where I forced a CI failure: #2421

Is this what you mean by the PR being "broken for other reasons?" Notice that GitHub indicates which action it was that failed. So in the screenshot you can see that the PR passed the lint check but failed the validation step because the OBOFoundry Test / test (pull_request) action has failed. If I combine linting and validation into a single action, a failure will be even more ambiguous (did it fail because the linter failed or the validation?). Is that what you were referring to? If not, I'm not really understanding the concern.

[matentzn]

As long as the QC is run AFTER the lint is applied, we are good! Is that the case?

[nlharris]

@erik-whiting did you see Nico's question?

[erik-whiting]

I saw his question and took some initial stabs at making it the way Nico wants but I have been really busy with work and school

[nlharris]

Ok, no worries! You do a ton of volunteer work for OBO Foundry and we are grateful for whatever you have time for. 🙏

[matentzn]

@erik-whiting until you find some time to respond (good luck with your stuff) I will move this PR to "draft"

[jsstevenson]

Have spent some time thinking about this on and off over the past few weeks. I think that maybe the solution which makes everyone happy is something like: an auto-formatter action that runs the tox format task, then runs the tox QA task. If the QA task passes, it commits, but if it fails, then the Action reports a failure (probably the user has accidentally submitted some malformed YAML or something). My outstanding question, for my own reference, is how to deal with this annoying "waiting for status to be reported" hangup -- if tests can't run from GitHub Actions-created PRs, I'd rather they don't show up at all, rather than showing up in an "infinite deferral" status.

[matentzn]

What if the QC check:

1. first runs linter
2. commit changes if any
3. runs qc

that way the trigger is the usual (manual curation), and the qc is just "the last step" of the QC pipeline.