Updating ROS2-Doc Cotr. Guildelines

[Nils-ChristianIseke]

After switching to .. code-block:: console we should update the cont. guidelines.
Only .. code-block:: console should be used for commands.

Addded a note (for outputs containing #) instead of an example, as I feel the contr. guidelines are already too long.

[fujitatomoya]

Only .. code-block:: console should be used for commands.

why we need to enforce this? i personally would like to keep the flexibility that i can choose. and i think nothing wrong is bash with no $ if that is the case, correct? that is actually provided as supported tag. or do we have any downside to use bash with no $?

how about keeping this preference or recommendation with extra note on line 491? so that people can choose either of them?

[christophebedard]

Only .. code-block:: console should be used for commands.

why we need to enforce this? i personally would like to keep the flexibility that i can choose. and i think nothing wrong is bash with no $ if that is the case, correct? that is actually provided as supported tag. or do we have any downside to use bash with no $?

how about keeping this preference or recommendation with extra note on line 491? so that people can choose either of them?

I think the benefits would be: (1) it makes it clear that the given command needs to be run in a terminal and (2) it's consistent, i.e., there's only 1 way to present a terminal command. However, I'm personally fine with keeping it as-is (bash or console) if you prefer the flexibility.

[Nils-ChristianIseke]

I'd love it if we only have the console as valid option for commands. But I also don't understand what advantage flexibility would have here: it means one more degree of freedom for the person writing and reading this and potentially leads to confusion. At least it would have confused me a few years ago when I came across ROS without any background in programming. And the goal of the PRs marathon was to enforce console :)

[fujitatomoya]

@christophebedard @Nils-ChristianIseke thanks for the comments. i am just not convinced why we do this...

But I also don't understand what advantage flexibility would have here: it means one more degree of freedom for the person writing and reading this and potentially leads to confusion.

i would bring this topic to Sphinx documentation project if i were you, because that is not us to provide those options? i just do not understand why we maintain this special constraint by ourself? if there is not strong justification, i prefer the less rules (avoid specific rules) for maintainability and maintainers.

And the goal of the PRs marathon was to enforce console :)

if that is the plan, how do we enforce this rule. coming back to my original concern how we maintain this... do we need to develop and maintain the specific checker tool or something?

[christophebedard]

I don't want to push hard on this, but here are my thoughts:

i would bring this topic to Sphinx documentation project if i were you, because that is not us to provide those options? i just do not understand why we maintain this special constraint by ourself? if there is not strong justification, i prefer the less rules (avoid specific rules) for maintainability and maintainers.

I don't think this is a Sphinx thing. I think it's a Shinx user thing.

Sphinx offers bash for bash and console for a mix of terminal commands and output. I'd argue that we are using Sphinx wrong if we're using bash for commands meant to be run in a terminal. However, if the code-block only has commands, then using a bash code-block does work and is simpler. But again, we are using Sphinx wrong.

Code-block consistency (i.e., always using console with $ for terminal commands) aside, issues arise when someone wants to add some output lines to an existing bash code-block: they add $ to the command lines and might forget to change to a console code-block, which has non-critical but non-nice results (i.e., the $ can be selected and all the text, including the output lines, gets copied with the copy button).

So I don't see it as a special or unnecessary constraint/rule. I see it as a way to simplify the instructions and reduce the potential issues, as mentioned above.

if that is the plan, how do we enforce this rule. coming back to my original concern how we maintain this... do we need to develop and maintain the specific checker tool or something?

It would be great to have this, but I don't think it should stop us from making this change to our instructions/guidelines. I can try to look into ways to specifically check Sphinx code-block directives using bash and report an error if they use $.

---

If we can't reach a consensus, then let's just keep the note about # and consider making the change to the guidelines later, if/when I can find or write a linter for it.

[Nils-ChristianIseke]

@fujitatomoya friendly ping :) If you disagree i will modify this PR, happy to get this closed soon.

[fujitatomoya]

@Nils-ChristianIseke ah sorry i missed this.

Sphinx offers bash for bash and console for a mix of terminal commands and output. I'd argue that we are using Sphinx wrong if we're using bash for commands meant to be run in a terminal.

i think this makes sense.
i thought that this bash would be useful for single liner bash command to highlight the bash syntax. i checked this with my local environment, but i do not see the differentiating syntax highlight than console.

but I don't think it should stop us from making this change to our instructions/guidelines. I can try to look into ways to specifically check Sphinx code-block directives using bash and report an error if they use $.

i agree with this. i mean, we actually had a rule single sentence per line for a long time without having check script 😓 (this is why there were many comments that explains single sentence per line for each review. i personally want to avoid this situation for all maintainers.)

i am okay to take this, but personally i would not put the enforcement rule without having the capability to check that in CI. i am almost sure that there is going to be inconsistency (some people rely on this inconsistency) and maintenance cost for patrol... until we figure out the checker.

[Nils-ChristianIseke]

i am okay to take this, but personally i would not put the enforcement rule without having the capability to check that in CI. i am almost sure that there is going to be inconsistency (some people rely on this inconsistency) and maintenance cost for patrol... until we figure out the checker.

Just to confirm, are you ok (despite some reservations) to merge it this way? I believe that in reviews, the reviewer only needs to mention it once rather than commenting on every example? See #5573 for an example.

[fujitatomoya]

@Nils-ChristianIseke

I believe that in reviews, the reviewer only needs to mention it once rather than commenting on every example? See #5573 for an example.

can you rephrase this? what is that supposed to mean?

i am not positive on this, but i think that is okay to merge. but let me bring this enforcement rule consensus for next ROS PMC meeting for the agreement before merge. does that make sense?

[Nils-ChristianIseke]

can you rephrase this? what is that supposed to mean?

Sure :). I would assume that your concern about review noise— pointing out the misuse of bash if we were to use this rule—might not be a significant issue in practice. As you could point it out once for each PR. Actually, I would assume that the suggested guideline might reduce review noise overall, since it would lead to a simpler and clearer contribution guideline regarding bash/console.

That said thank you for bringing it to the next PMC Meeting.

Sorry for pushing on this—just hoping to get some of my stale PRs closed :).

[fujitatomoya]

no worries, thanks for explanation and your patience.

Actually, I would assume that the suggested guideline might reduce review noise overall, since it would lead to a simpler and clearer contribution guideline regarding bash/console.

yeah that is where our assumption becomes different... 😅 (i do not want to talk in negative...) i think some developers do not even read the guideline before contribution, and then i think this is gonna be highway-patrol thing... maintainers need to keep eyes on this rule. and if we miss, there is a guideline but actually that is not how implemented, eventually inconsistent situation.

actually looking at https://docs.ros.org/en/rolling/The-ROS2-Project/Contributing/Contributing-To-ROS-2-Documentation.html#writing-pages and this PR, it does not say this is the strict rule to take the PR? that means this PR still can be kind of preference.

[christophebedard]

Suggested change

[fujitatomoya]

revived by de29881

[fujitatomoya]

@Nils-ChristianIseke we have made the vote to keep the policy in preference, i applied de29881 on your dev branch.

@christophebedard can you review and merge this?

[christophebedard]

LGTM! Thanks for following up.