OBOFoundry.github.io icon indicating copy to clipboard operation
OBOFoundry.github.io copied to clipboard

Add action to automatically update PRs with tox -e lint

Open matentzn opened this issue 2 years ago • 2 comments
trafficstars

Its annoying to do this manually - this should be done automatically.

The github workflows in this repo have enough detail to figure out how to do this probably.

Here is an example of how in OBA we commit changes to a file using GH action: https://github.com/obophenotype/bio-attribute-ontology/blob/master/.github/workflows/dosdp.yml#L26

matentzn avatar May 17 '23 13:05 matentzn

Are you saying you want tox run on any pull requests once they're submitted? If so, I have a suggestion, because there's a small problem that will come with that.

Say you submit a PR, and this worker automatically runs tox. The worker then commits the changes to the PR to save them. Now say someone asks you to change something on your PR. You make the small change locally and then try to push, but git rejects your push because your local branch is now out of sync with the remote branch.

As soon as any worker makes a commit on a remote branch, all local branches tracking it are now out of sync. You have to have the wherewithal to make sure you pull the upstream branch before making your changes. You could of course do a git push --force as the error message will likely suggest you do, or you could stash your changes, pull, and then unstash the changes once yoru local is resynced with remote. You could also delete your local branch and re-pull the remote branch.

For you and me, these are likely non-issues as we are quite familiar with git and have probably resynced our local branches 1,000 times this year alone. But I think not everyone involved in this project is so comfortable with git, and a warning message telling someone to use a flag called "force" is probably scary. For this reason, I don't really like this suggestion.

What I suggest instead is running tox and applying the changes immediately before merge. That is, once a PR has been approved and we click merge, the CI runs tox -e lint, commits the changes if there are any, and then goes on with the rest of its workflow. That way, the changes are already approved and that branch is about to be useless anyway, so no need to worry about out-of-sync branches.

How do you feel about this instead?

erik-whiting avatar May 18 '23 00:05 erik-whiting

This would mean, however, that the lint check (not the operation of applying the linting, but the test that checks if the file is well formatted) cant happen.. Will it not be exceedingly difficult to separate formatting issues that will be solved by running tox -e lint from the ones that won't be fixed by it?

I feel like 99% of the people use the GitHub user interface to change metadata. They wouldn't notice the updated branch.

The best would be a more deliberate "fix formatting issues" button that could be run by the person submitting the updated file, but I don't think that is possible..

matentzn avatar May 18 '23 09:05 matentzn