Trigger black workflow with `workflow_run`

[asidhu0]

The initial setup for the isort and black workflows was that isort would first run to completion and as soon as black detected that isort has finished execution black would begin to execute. This behavior was achieved using workflow_run, however, it resulted in unintended behavior on pull requests since workflow_run changes the state of GITHUB_REF to the default branch, which is main. So black was only running on the main branch regardless if it was triggered by a pull request.

Now, I am using workflow_call, which indicates that a workflow can be called by another workflow. This mechanism sets GITHUB_REF to the same value as it was in the caller workflow. So now isort runs first and triggers black to run and black runs on the active pull request branch since GITHUB_REF is the same as it was in the caller workflow.

Also I have modified cameras.py so that both isort and black should make formatting changes in the open pull request to ensure this behavior happens.

[russelldj]

Another question for my understanding: how did you get this workflow to run before it was merged? My assumption was that only the workflows on main were run.

[asidhu0]

Another question for my understanding: how did you get this workflow to run before it was merged? My assumption was that only the workflows on main were run.

Oh no, I thought we decided to have the workflows run on an active pull request and a push event to main. So the workflow would be triggered whenever someone would create a pull request or push to an existing pull request. In all the previous pull requests this has been the behavior with isort, but not black. But after integrating this pull request this behavior will be consistent for both isort and black.

[russelldj]

Another question for my understanding: how did you get this workflow to run before it was merged? My assumption was that only the workflows on main were run.

Oh no, I thought we decided to have the workflows run on an active pull request and a push event to main. So the workflow would be triggered whenever someone would create a pull request or push to an existing pull request. In all the previous pull requests this has been the behavior with isort, but not black. But after integrating this pull request this behavior will be consistent for both isort and black.

Sorry, that was confusing. Yes, I agree that workflows should execute on both PRs and pushes to main.

My question was about this recent update. It seemed like you successfully ran the workflow that was on this PR (and added both black and isort changes) but not yet merged to main. I was confused how you did that, since I expected that only the workflow folder in main was used for running workflows on all branches. Is that assumption incorrect?

And sorry for initially editing your comment, I accidentally did that when I meant to reply.

[asidhu0]

Another question for my understanding: how did you get this workflow to run before it was merged? My assumption was that only the workflows on main were run.

Oh no, I thought we decided to have the workflows run on an active pull request and a push event to main. So the workflow would be triggered whenever someone would create a pull request or push to an existing pull request. In all the previous pull requests this has been the behavior with isort, but not black. But after integrating this pull request this behavior will be consistent for both isort and black.

Sorry, that was confusing. Yes, I agree that workflows should execute on both PRs and pushes to main.

My question was about this recent update. It seemed like you successfully ran the workflow that was on this PR (and added both black and isort changes) but not yet merged to main. I was confused how you did that, since I expected that only the workflow folder in main was used for running workflows on all branches. Is that assumption incorrect?

And sorry for initially editing your comment, I accidentally did that when I meant to reply.

Oh, I understand now, that's a good point. I did not think of this before and the newly updated workflow ended up running. Looking at github discussions it seems like there is no solid answer in the documentation. I think maybe what happened was since the workflow gets triggered on a pull_request it runs the workflow file from the pull request. I saw something about how if it was a pull request from a fork it would run the workflow file from main for security concerns.

[asidhu0]

Nice job figuring this out and thanks for testing it. Just minor feedback on the PR itself, but bigger picture comments below.

It was completely reasonable to make changes that would be reverted, especially since I know that you tested this on another repo previously. This is definitely a nitpick, but in the future I'd recommend that you do it in a way where we can revert the changes made for testing so they don't show up in the commit history. In this case, you could have split the changes to the workflow files and the changes to cameras.py into two commits, with the changes to cameras.py being the second one. Then, push those changes and let the workflow run. Once it has run successfully, you could use git reset --hard <commit ID of last non-testing commit> to reset your local branch to the state before testing and git push --force to overwrite the changes on the remote. Alternatively, if you did more useful changes after the testing commits, you can use git rebase -i to drop commits from the history and then again use git push --force to overwrite the remote.

Don't worry about this for this PR, but maybe give it a try on your demo repo if you have time.

This makes a lot of sense and would be much better to do, thanks for letting me know. I made those changes because I knew they would get reverted immediately and so in the pull request the useless changes would not appear. However, I disregarded that they would still appear in the commit history.

[asidhu0]

Also now in the Github actions tab only one workflow will show:

I changed the name field in the isort yaml file to Code Formatter so that in the Github Actions tab it shows that a workflow is running to format code. See in the picture above below the commit message.

However, this leads to some ambiguity since the file with name = Code Formatter is formatting code based on isort and just calling the black yaml file. So in the side bar we have this view:

Would you like me to change the naming convention? I can also change it so that there is just one yaml file that runs both isort and black.

[russelldj]

Also now in the Github actions tab only one workflow will show:

I changed the name field in the isort yaml file to Code Formatter so that in the Github Actions tab it shows that a workflow is running to format code. See in the picture above below the commit message.

However, this leads to some ambiguity since the file with name = Code Formatter is formatting code based on isort and just calling the black yaml file. So in the side bar we have this view:

Would you like me to change the naming convention? I can also change it so that there is just one yaml file that runs both isort and black.

I think this is good as is. If we start adding more workflows, maybe we should merge the two.