Closed simonschmeisser closed 2 years ago
Thanks for the PR. Have you run this on your fork?
I would expect catkin_lint
to not be too happy fi.
I just figured out how to run this manually, let's see what results we get. Would you prefer (temporarily) disabling catkin_lint
or fixing it within this PR?
I would suggest to tackle one thing at a time (so not enable catkin_lint
just yet).
Looks like this passes now:
Feel free to merge @simonschmeisser :+1:
Let's not squash merge this so as to preserve the separate commits.
@simonschmeisser: don't forget to re-enable branch protection again after this is merged and require the GHA status check.
If you enable to admin-exception, you should still be able to merge PRs which don't have it.
Thanks for the quick review! Am I correct in assuming that "Rebase and Merge" is prefered over "Create a merge commit"?
You already disabled branch protection right?
Am I correct in assuming that "Rebase and Merge" is prefered over "Create a merge commit"?
I'd suggest to use regular merges whenever possible. That way provenance of commits is retained as much as possible and we don't really see that much PRs here to warrant going for rebases by default.
If / when it makes sense, rebases can be used.
You already disabled branch protection right?
To be able to merge this one, yes.
Sorry, you were too quick ... but now the commits are clean
Thanks :+1:
I've kicked off the actions.
I either cannot find the branch protection settings or lack the privileges to see/change them
Mentioning https://github.com/ros-industrial/ros_industrial_issues/issues/74 here so we keep things connected.
This was based on the respective changes in the fanuc repository