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 Humble to REP 2000 #326

Merged
merged 7 commits into from
Nov 18, 2021
Merged

Add Humble to REP 2000 #326

merged 7 commits into from
Nov 18, 2021

Conversation

audrow
Copy link

@audrow audrow commented Nov 9, 2021

This PR adds an entry for Humble Hawksbill to REP 2000.

I tried to grab the relevant dependencies. For Ubuntu Jammy and Windows 10. These may change before Humble is released. Also, I've left RHEL 8 in, since it is currently supporting Rolling.

Signed-off-by: Audrow Nash <audrow@hey.com>
@audrow audrow self-assigned this Nov 9, 2021
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

Overall, this looks like a good start. I've left a couple of comments for improving things.

rep-2000.rst Outdated Show resolved Hide resolved
rep-2000.rst Outdated Show resolved Hide resolved
rep-2000.rst Outdated Show resolved Hide resolved
paudrow and others added 3 commits November 10, 2021 13:27
Signed-off-by: Audrow Nash <audrow@hey.com>

Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Audrow Nash <audrow@hey.com>
Signed-off-by: Audrow Nash <audrow@hey.com>
@audrow audrow requested a review from clalancette November 10, 2021 21:34
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks good to me now. I'd actually be inclined to add a note to the top of the new section saying "this is subject to change until April 2022", and then merge this PR in now. @audrow what do you think?

Signed-off-by: Audrow Nash <audrow@hey.com>
@audrow
Copy link
Author

audrow commented Nov 11, 2021

This looks good to me now. I'd actually be inclined to add a note to the top of the new section saying "this is subject to change until April 2022", and then merge this PR in now. @audrow what do you think?

Sounds good to me. I added a note and linked to the release timeline.

@audrow audrow requested a review from clalancette November 11, 2021 18:22
@audrow audrow marked this pull request as ready for review November 11, 2021 18:23
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks good to me, but let's keep this open for a few more days to collect any more feedback.

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/updating-rep-2000-for-humble-hawksbill/23077/1

Copy link
Contributor

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm

@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/preparing-ros-2-rolling-for-the-transition-to-ubuntu-22-04/23124/1

Copy link
Contributor

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

One small update to empy on Ubuntu but otherwise looks good.

rep-2000.rst Outdated Show resolved Hide resolved
@nuclearsandwich
Copy link
Contributor

nuclearsandwich commented Nov 16, 2021

Signed-off-by: Audrow Nash <audrow@hey.com>

Co-authored-by: Steven! Ragnarök <nuclearsandwich@users.noreply.github.com>
@audrow
Copy link
Author

audrow commented Nov 16, 2021

Oh, and @audrow consider adding yourself to the authors list https://github.com/ros-infrastructure/rep/blame/f99ff26309760947501c0927be1f4fbdafb6f10d/rep-2000.rst#L3

I didn't add myself because I didn't see previous ROS Bosses on there. @clalancette, do you think I should add myself as an author (and maybe the previous ROS Bosses, as well)?

@clalancette
Copy link
Contributor

I didn't add myself because I didn't see previous ROS Bosses on there. @clalancette, do you think I should add myself as an author (and maybe the previous ROS Bosses, as well)?

I have no strong opinion on it. I'd just add yourself if you'd like to, but not add anyone else. Also, I would suggest updating the date at the top of the document. Once those two things are done, I'll suggest going ahead and merging this.

Signed-off-by: Audrow Nash <audrow@hey.com>
@audrow audrow merged commit eae0fb5 into master Nov 18, 2021
@audrow audrow deleted the audrow/update-2000 branch November 18, 2021 17:52

Minimum language requirements:

- C++17
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason not to target c++20?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason not to target c++20?

There are a few reasons we don't think we are going to do this for Humble:

  1. Every time we've switched C++ versions, it has taken some non-trivial amount of work to keep things working without warnings. Someone would have to step up to do that work.
  2. All of our Tier 1 platforms probably support C++20 well, but not all of the Tier 2 and Tier 3 ones do.
  3. Part of the ROS 2 community deals with safety certification, things like MISRA. Since those standards tend to lag behind the relevant standards, switching to a new C++ version too early can leave those users behind.

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.

9 participants