grid_map icon indicating copy to clipboard operation
grid_map copied to clipboard

Merge Master into ROS2

Open remod opened this issue 4 years ago • 11 comments

Merging fixes from master into the ros2 branch.

remod avatar Jul 07 '20 06:07 remod

@remod where did you branch ros2 from? It looks like you have some merge conflicts and your branch was older than when we setup CI so CircleCI is failing.

You could cherry-pick your commit range over or rebase.

SteveMacenski avatar Jul 07 '20 06:07 SteveMacenski

Hm, it only has the changes it should have.

So CircleCi fails with:

#!/bin/sh -eo pipefail
# No configuration was found in your project. Please refer to https://circleci.com/docs/2.0/ to get started with your configuration.
# 
# -------
# Warning: This configuration was auto-generated to show you the message above.
# Don't rerun this job. Rerunning will have no effect.
false

This looks to me more like a basic issue.

maximilianwulf avatar Jul 07 '20 07:07 maximilianwulf

That's why I'm saying I think its a base issue. There's even merge conflicts, I think their local version was really old. They should cherry pick or rebase.

SteveMacenski avatar Jul 07 '20 07:07 SteveMacenski

@SteveMacenski master has got some fixes, and this PR just merges master into the ros2 branch. As the ros2 branch underwent a complete reformatting this leads to merge conflicts now and every time we want to update the branch with master. Is this the desired workflow? Does it make sense to reformat master as well?

remod avatar Jul 14 '20 10:07 remod

Perhaps, yes. I do think trying to cherry pick the commits over one by one would be fine.

SteveMacenski avatar Jul 14 '20 16:07 SteveMacenski

Cherry-picking leads to the same merge conflicts, as you cannot apply commits 1-1 if the formatting changed...

remod avatar Jul 15 '20 07:07 remod

if you cherry-picked locally, you would have been asked to resolve these conflicts in the terminal. Since they appear here, I don't think you did that correctly. I see a marge into master rather than a cherry pick on a clean branch.

SteveMacenski avatar Jul 15 '20 19:07 SteveMacenski

Yes, it's clear that I can resolve the conflicts locally. However, it does not seem scale well if we have merge conflicts with every little fix we do on master. Does it make sense to reformat master as well?

remod avatar Jul 16 '20 10:07 remod

We could certainly do that, yes. The uncrustify errors can be automatically reformatted which will do the bulk of the work, but there's probably a solid day of linting for the cpplint compliance.

Keep in mind you'll still have merge conflicts regardless because the ROS APIs and structure have changed so much, but it would be lessor

SteveMacenski avatar Jul 16 '20 18:07 SteveMacenski

Congratulations :tada:. DeepCode analyzed your code in 2.666 seconds and we found no issues. Enjoy a moment of no bugs :sunny:.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

👉 The DeepCode service and API will be deprecated in August, 2021. Here is the information how to migrate. Thank you for using DeepCode 🙏 ❤️ !

If you are using our plugins, you might be interested in their successors: Snyk's JetBrains plugin and Snyk's VS Code plugin.

ghost avatar Sep 11 '20 17:09 ghost

I've marked this for 2.2.0 milestone, but needs an overhaul to only include the critical bugfixes. The rest can be kicked out to a later release.

Ryanf55 avatar Feb 03 '24 01:02 Ryanf55