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

Version number of master #2036

Closed
simonschmeisser opened this issue Apr 24, 2020 · 11 comments · Fixed by #2123
Closed

Version number of master #2036

simonschmeisser opened this issue Apr 24, 2020 · 11 comments · Fixed by #2123
Labels

Comments

@simonschmeisser
Copy link
Contributor

I'm somewhat confused about the version numbers in master, shouldn't they be bigger than in melodic-devel? I would have expected something like 1.1.0 or similar? I was trying to write an #ifdef but that kind of makes it hard?

Tagging the usual suspects @v4hn @rhaschke

@rhaschke
Copy link
Contributor

The problem is that the ROS community follows the policy to increase the version number just before a new release. Hence, the master is usually has the same version number as the latest release. For MTC we use this cmake snippet to decide for master:

# check for MOVEIT_MASTER
include(CheckIncludeFileCXX)
set(CMAKE_REQUIRED_INCLUDES ${moveit_core_INCLUDE_DIRS})
set(CMAKE_REQUIRED_FLAGS "-std=c++11")
CHECK_INCLUDE_FILE_CXX(moveit/collision_detection/collision_env.h MOVEIT_MASTER)
if(NOT MOVEIT_MASTER)
  set(MOVEIT_MASTER 0)
endif()
add_definitions(-DMOVEIT_MASTER=${MOVEIT_MASTER})

@simonschmeisser
Copy link
Contributor Author

Hmm, maybe it should be discussed at the next meeting if MoveIt should continue following that policy or have the "next" releases version number in master already

@rhaschke
Copy link
Contributor

All the available release tools don't support this scheme very well.

@simonschmeisser
Copy link
Contributor Author

but master is not currently released right?

@rhaschke
Copy link
Contributor

Correct. At some point we will create a release branch from master and release it into Noetic.

@simonschmeisser
Copy link
Contributor Author

My idea was to now set master to 1.1.0 and then later when we branch noetic-devel set master to 1.2.0 and noetic-devel to 1.1.1 just before release. Would that still break somewhere? (Except for not having a 1.1.0 released ever?)

@v4hn
Copy link
Contributor

v4hn commented Apr 27, 2020

Indeed, version checks would clearly be better than our file-check workaround in MTC.

@rhaschke is clearly the expert on the release tools (highlighting @JafarAbdi for their opinion, too), so whatever they say. :)

The scheme you propose seems feasible to me and I would support it for MoveIt unless bloom will fail if we touch the version number outside the full release process.

@rhaschke I guess we could bump the version number of all packages by hand for the master branch? After splitting off?

The general scheme to bump MINOR/PATCH version after a release for every branch would be annoying overhead with bloom, I guess, although it's standard practice in many OpenSource packages outside ROS.

@simonschmeisser
Copy link
Contributor Author

@rhaschke are there technical problems with bloom besides "wasting" one version number that never gets released? I have two places where this would be handy by now

@rhaschke
Copy link
Contributor

I just tested the procedure. In principle, we can use the available tools to realize your goal of having the upcoming release version already indicated at the master branch as follows:

# after a release: bump the release number, ignore warnings about missing changelogs:
catkin_prepare_release --no-push 
# manually reword the commit message:
git commit --amend -m "Bump version to x.x.x"

#... a lot of commits for the next release ...

# before release, generate the changelog as usual
catkin_generate_changelog
# augment the release number in all changelogs:
catkin_prepare_release --version x.x.x
bloom

We wouldn't even "waste" release numbers. However, this procedure needs to be documented and followed by all release maintainers! Let's discuss this at tomorrow's meeting.

Any comments @v4hn, @gavanderhoorn, @davetcoleman?

@v4hn
Copy link
Contributor

v4hn commented May 27, 2020

Sounds like a very good and even rather straight-forward best-practice.

@mamoll
Copy link
Contributor

mamoll commented May 28, 2020

The general consensus from the Moveit Manipulation WG meeting was that this is a good idea. 👍 from me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants