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

add CMake macro ament_bump_development_version_if_necessary #204

Merged

Conversation

dirk-thomas
Copy link
Contributor

@dirk-thomas dirk-thomas commented Oct 16, 2019

Provide a standardized mechanism to override the exported package number. It can be used when a package makes breaking changes and wants to already identify itself with the upcoming version number to enable conditional logic.

The -dev suffix if a pure recommendation and not strictly necessary.

Regarding the naming of the macro please make concrete proposals if you would like to see a different name.

Generating a C header and providing C macros to compare version numbers is explicitly not included in this PR since it is considered syntactic sugar and not required to satisfy ros2/rmw#188.

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas added enhancement New feature or request in review Waiting for review (Kanban column) labels Oct 16, 2019
@dirk-thomas dirk-thomas self-assigned this Oct 16, 2019
Copy link
Contributor

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the macro is called more than once? We had discussed (offline) an error if it was called more than once with conflicting versions.

In general, the change is ok for me.

I don't think we need a new package for it, since it adds no appreciable complexity nor any new dependencies, so I feel like it could just go in ament_cmake itself...

@dirk-thomas
Copy link
Contributor Author

What happens if the macro is called more than once? We had discussed (offline) an error if it was called more than once with conflicting versions.

Good question. I was iterating on this while I implemented it. The scenario we considered earlier was two calls being in the code base - potentially in different conditional sections. They might both use the same development version or not. And I think in the case where they don't, the function doesn't have to result in a fatal error. Instead what will naturally happen is that the exported version is the higher of multiple calls. And I would expect that being the intention since each call in a conditional scope "raises the bar" for a certain reason.

I don't think we need a new package for it, since it adds no appreciable complexity nor any new dependencies, so I feel like it could just go in ament_cmake itself...

ament_cmake is a metapackage not containing any functionality so I guess adding it there isn't desired. Imo it certainly doesn't belong in ament_cmake_core. Therefore I think a new package is the right choice. In the future this package can also contain the functionality proposed in #198.

@wjwwood
Copy link
Contributor

wjwwood commented Oct 17, 2019

And I would expect that being the intention since each call in a conditional scope "raises the bar" for a certain reason.

Ok fine with me.

dirk-thomas and others added 5 commits October 17, 2019 14:00
…cessary.cmake

Co-Authored-By: William Woodall <william@osrfoundation.org>
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
…cessary.cmake

Co-Authored-By: William Woodall <william@osrfoundation.org>
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas dirk-thomas force-pushed the dirk-thomas/ament_bump_development_version_if_necessary branch from d634394 to b87b4e6 Compare October 17, 2019 21:00
@dirk-thomas
Copy link
Contributor Author

Rebased for the necessary Signed-off-by.

@dirk-thomas
Copy link
Contributor Author

dirk-thomas commented Oct 17, 2019

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@dirk-thomas dirk-thomas merged commit cd05eda into master Oct 17, 2019
@delete-merged-branch delete-merged-branch bot deleted the dirk-thomas/ament_bump_development_version_if_necessary branch October 17, 2019 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in review Waiting for review (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants