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

Switch the testing matrix to only test on humble. #146

Closed
wants to merge 1 commit into from

Conversation

clalancette
Copy link
Contributor

Also make one small fix the README while we are here.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

Also make one small fix the README while we are here.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette requested a review from mintar May 27, 2022 14:13
@mintar
Copy link
Contributor

mintar commented Jun 2, 2022

In #138, @clalancette wrote:

I'll go ahead and make the branch, though I'm not entirely sure we should keep them synced and using the same matrix. That is, it would be simpler to just have the humble branch test against humble, and the rolling branch against rolling. It requires a bit more work to backport everything, but I think it is much simpler for users to understand.

I don't think it's simpler for users to understand. Inevitably, we'll forget to back-port some PRs, and they'll be back-ported later out of order. Is it really simpler to see at a glance if the code between the two branches is identical if we let them diverge for no real reason? So my argument is that it's not only more work for us but also more confusing for users. All repos I've seen that used this "let it diverge" approach have had similar problems - PRs forget to be back-ported, maintainers lose track of what's back-ported and what isn't, bugs get hard to debug because you have so many different versions of your software that you're trying to maintain at the same time, and because it's so much work for the maintainers, support for older LTS ROS versions is often dropped prematurely.

In the past, I've had very good experiences with the following approach:

  • Create two branches (here: humble and rolling). This is useful even if they are identical at the start because when they diverge later, all users already have checked out the appropriate branch and won't get any updates that are meant to be only for a later ROS version. It also means that we don't have to update the branch name in the rosdistro repo, because the semantics of the branch never change - humble will always remain the branch for humble.
  • Run CI for both ROS versions on the same branch. This ensures that a PR for one ROS version also works on the other.
  • Keep the branches in sync manually by simply doing fast-forward merges. Much easier than cherry-picking between the branches. Both branches will always point to the same commit (so far).
  • When a breaking change occurs that must only be applied to the newer ROS branch, that's the moment when I let the branches diverge. For example, 43f4169 is an example of such a breaking change. Up until that commit, noetic and melodic were in sync. At that point, the CI config should also be changed so that future commits are only tested against the newer (resp. older) ROS distro.
  • After that point, I'll cherry-pick or open back-porting pull requests between both branches.
  • At some point, I stop supporting the older ROS distro and just leave the branch alone.

Of course, all of that is only a suggestion. If you prefer to do it your way, that's fine too!

@mintar
Copy link
Contributor

mintar commented Jun 17, 2022

@clalancette : Any opinions? If you really prefer branching off now, we can do that. It's up to you.

@clalancette
Copy link
Contributor Author

@mintar Sorry for the delay here.

I think your approach is the best of both worlds; we branch off (so it is easy to tell at a glance that the humble branch belongs to Humble), but we also ensure that backports are done to all supported branches simultaneously.

So with that in mind, then, I think what we would do here is to close this PR and instead open one that targets rolling, and adds in humble to the test matrix. Once we merge that, we would fast-forward that to the humble branch, and then going forward we would continue doing that unless/until we need to do some breaking change. Does that match with your understanding?

@mintar
Copy link
Contributor

mintar commented Jun 20, 2022

Yes, that matches my understanding exactly!

@clalancette clalancette deleted the clalancette/humble-update-matrix branch June 20, 2022 14:38
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.

2 participants