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 Dashing 'interface show' syntax back to tutorials #685

Merged
merged 5 commits into from
May 20, 2020

Conversation

maryaB-osr
Copy link
Contributor

People still use Dashing

Didn't change the Actions tutorials, will do in the open PR for Actions: #594

Signed-off-by: maryaB-osr [email protected]

@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-685 May 12, 2020 22:47 Inactive
Signed-off-by: maryaB-osr <[email protected]>
@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-685 May 12, 2020 22:51 Inactive
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.

Looks generally good. One thing I'd prefer is if we can mark the "Eloquent or newer" tab as the default one selected. Do you know how to do that?

Signed-off-by: maryaB-osr <[email protected]>
@maryaB-osr
Copy link
Contributor Author

@clalancette it just depends on what order they're in, first is the default

@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-685 May 13, 2020 19:51 Inactive
@clalancette
Copy link
Contributor

@clalancette it just depends on what order they're in, first is the default

Ah, that's unfortunate. What have we done elsewhere? I think it is important that we be consistent across the tutorials.

@maryaB-osr
Copy link
Contributor Author

I think I'll open another PR to make these consistent. Not only the order (some tutorials have older-first, some have newer-first) but also the wording (we have "Eloquent and newer", "Eloquent or newer", "Eloquent+", "Eloquent/Foxy", etc). I guess I can branch off of this branch to do that (unless you think this should be merged first)

@clalancette
Copy link
Contributor

I think I'll open another PR to make these consistent. Not only the order (some tutorials have older-first, some have newer-first) but also the wording (we have "Eloquent and newer", "Eloquent or newer", "Eloquent+", "Eloquent/Foxy", etc). I guess I can branch off of this branch to do that (unless you think this should be merged first)

I think it makes sense to do it all in here, since we are dealing with the tabs in this PR.

@mjcarroll mjcarroll temporarily deployed to ros2-documentation-pr-685 May 14, 2020 17:15 Inactive
@maryaB-osr
Copy link
Contributor Author

They're all left-to-right foxy-to-dashing now, and just use "/" if it applies to more than one distro.

So if it's something common to Foxy and Eloquent, it'll be

Foxy/Eloquent | Dashing

And if it's common to Eloquent and Dashing but Foxy is different, then:

Foxy | Eloquent/Dashing

@clalancette
Copy link
Contributor

So if it's something common to Foxy and Eloquent, it'll be

Foxy/Eloquent | Dashing

And if it's common to Eloquent and Dashing but Foxy is different, then:

Foxy | Eloquent/Dashing

I probably should have thought of this earlier, but this is going to cause us to have to revisit all of these tabs for G-Turtle as well. I think I'd prefer something more like:

Eloquent and newer | Dashing

or:

Foxy and newer | Eloquent/Dashing

That is, we assume that things will remain the same going forwards, but we call out the differences to the still supported versions. @maryaB-osr does that make sense to you?

@maryaB-osr
Copy link
Contributor Author

went with your suggestion @clalancette. i also removed a big section "crystal only" section, i think that's safe right?

@clalancette
Copy link
Contributor

went with your suggestion @clalancette. i also removed a big section "crystal only" section, i think that's safe right?

Yeah, we can remove things referring to Crystal and older, they are no longer supported.

@maryaB-osr maryaB-osr merged commit ff9b146 into master May 20, 2020
@delete-merged-branch delete-merged-branch bot deleted the maryaB/interface-show branch May 20, 2020 17:19
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.

3 participants