ci: add workflow to format whitespaces #236
Conversation
There was a problem hiding this comment.
I'm still not 100% convinced whether it is actually good idea to update user's branches on push, because it can cause unexpected merge-conflicts (user pushes a change, then does more code changes, and later before another push merge is necessary).
There is only one way to find out - lets give it a try 😄
I understand that this might be a concern but usually whitespace conflicts are handled well by git and a simple |
Co-authored-by: Marcin Krystianc <marcin.krystianc@gmail.com>
|
Welp @marcin-krystianc, it seems that GitHub doesn't allow workflows to write to forks even with "Allow edits and access to secrets by maintainers " enabled. This means that this approach won't work. I was testing this on a branch of the same repository, and assumed it would work on a fork. I apologise for wasting your time. I'll look into it a bit more before closing this PR. |
I don't think that everything is lost. Maybe the workflow for the fork can run directly in the forked repository, that would solve the problem with permissions? |
I think it's already doing that.. I'll have to play around with it more. |
|
Ok. Good news. I just tested this. It works. I created a GitHub organisation, setup the repository with the workflow, forked the repository, created a PR, and the workflow ran fine on my fork. There's still some issues left to smooth out. |
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
Funnily enough, what always worked was running the workflow on a |
|
It's not super ideal but I made it skip all non-fork repositories. |
|
I'll have a look, for the time being I just found this link which mentions that this automatic formatting solution will not work in all cases. |
This was the problem I faced but it seems to work for now in the current implementation. We aren't using As you can see here, it runs fine. |
|
Hi @marcin-krystianc, with some advice from @AdnaneKhan, I managed to get it to work with It will only work once this PR is merged, so we can't test it on this repository but you can see my tests on my own organisation/repository PR here. |
|
All right, it seems it works in bot cases, for PR from the original repository and from the forked repository as well. Nice job!! |
Summary
This PR adds a GitHub Action workflow that will, on every change to a
.csfile, trigger a workflow which will fix the whitespace of such files, and commit the produced changes. If no whitespace offenses are found, nothing is committed. The workflow was written to complete as quickly as possible to provide immediate user feedback.Reason
Even with EditorConfig configured, I often find that the whitespaces in my commits somehow fail the existing
check-formatworkflow. This is inconvenient as I have to commit the result fromdotnet tool run dotnet-formatmultiple times while working on a PR. If you check thegit diffof the offending commits, you will notice that they appear identical. Furthermore, we are only fixing the whitespace in this workflow which shouldn't introduce any logical bugs if the tool were to detect a false positive.TL;DR Free formatting at zero cost???