Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto Build #446

Merged
merged 5 commits into from
Jun 20, 2021
Merged

Auto Build #446

merged 5 commits into from
Jun 20, 2021

Conversation

MatthijsBurgh
Copy link
Contributor

@MatthijsBurgh MatthijsBurgh commented Jun 6, 2021

Instead of requiring the build to be up-to-date, GH actions will push the build folder when the CI procedure has resulted in a diff.

To test it, I added some commits to push on this feature branch. The commit done by GH actions: 5112758

GH actions run: https://github.com/RobotWebTools/roslibjs/runs/2758087742?check_suite_focus=true

I dropped these commits again and forced pushed. I can't guarantee this will work with the branch protection on develop. But merge this PR is the only way to find out. There is no other way to test this.

Once approved, I will apply this to ros3djs and ros3djs too.

@MatthijsBurgh
Copy link
Contributor Author

This will not work at the moment, because of the branch protection. https://github.com/RobotWebTools/roslibjs/runs/2773549430?check_suite_focus=true#step:6:50

@MatthijsBurgh
Copy link
Contributor Author

To get the it working with branch protection, either a personal acces token(PAT) should be used. Or we should allow force pushes. I don't like to allow force pushes. Leaving a PAT as the only option. As recommened by the author of the GH Action used to commit the build (https://github.com/stefanzweifel/git-auto-commit-action#push-to-protected-branches), it is better to use a seperate (bot) account for that. So anyone from @RobotWebTools/roslibjs-maintainers who wants to create/maintain this seperate account? Or do you disagree want to provide your account for this?

@nuclearsandwich
Copy link
Member

@MatthijsBurgh I'm still trying to fully understand the task at hand, but does it make sense to have the auto build action run on the pull request branch before it's merged into develop so that the HEAD commit of each PR is the auto-build commit? If you combine that workflow with a forward integration strategy (merging develop into the PR branch on each push/build), which can also be automated I think you can get the same result without performing automated pushes to the protected branch.

@MatthijsBurgh
Copy link
Contributor Author

That would have multiple downsides.

  • When you are working in a branch and you push. The remote will always add a commit and will be ahead of your local branch. A bit annoying IMO.
  • When someone outside the maintainers opens a PR, we can't always commit to their branch. So that would require us to ask them to update the build or ask them to allow us to push to their branch.
  • The new commit will not trigger a new run of the GH actions. On one side is this OK as the library did not change compare to the previous run. But because of this the PR test checks will not work. As the last commit will not have any tests.

@jihoonl
Copy link
Member

jihoonl commented Jun 11, 2021

I like the idea of pushing to the branch instead of develop branch. But, I'm not against to @MatthijsBurgh's opinion either. So, let's try it out first. We can always disable back if it doesn't work.

@MatthijsBurgh I have just created @RWT-bot account. Would you send me an email to jihoonlee.in at gmail dot com? So, I can send you account information. Or, what would be the good way of supporting this feature to get it done?

@MatthijsBurgh
Copy link
Contributor Author

The bot does require admin rights to commit to the protected branch. I have tested it on develop2. The commit which updates the build does trigger a new action run. Which is a bit unnecessary. As the only diff is in the generated build folder.

I have added an if, so the jobs don't run on pushes by the RWT-bot. This doesn't influence the branch status, as an run is triggered. It just doesn't have any (failed) jobs, so it succeeds.

If you agree with this workflow, please approve.

@jihoonl
Copy link
Member

jihoonl commented Jun 19, 2021

Looks good to me. Let's give it a try.

@MatthijsBurgh MatthijsBurgh merged commit b654415 into develop Jun 20, 2021
@MatthijsBurgh MatthijsBurgh deleted the auto_build branch June 20, 2021 06:33
k-aguete pushed a commit to k-aguete/roslibjs that referenced this pull request Oct 21, 2022
Bumps [rollup](https://github.com/rollup/rollup) from 2.57.0 to 2.58.0.
- [Release notes](https://github.com/rollup/rollup/releases)
- [Changelog](https://github.com/rollup/rollup/blob/master/CHANGELOG.md)
- [Commits](rollup/rollup@v2.57.0...v2.58.0)

---
updated-dependencies:
- dependency-name: rollup
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants