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

Enhance contribution setup docs with streamlined commands #4181

Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from

Conversation

Victor-Boyd
Copy link

As an individual contributor to this open-source project, I noticed an opportunity to enhance the contributor experience by simplifying the initial setup instructions within our documentation. This commit aims to lower the entry barrier for new contributors by providing a concise, all-in-one command sequence that facilitates the setup process.

Key Updates:

  • Introduced a command for cloning the repository, ensuring newcomers start off on the right foot without having to navigate through multiple setup steps.
  • Integrated the PATH environment variable update (~/.local/bin) within the setup commands to eliminate the need for manual configuration, making the environment preparation seamless.
  • Combined cloning, environment setup, and dependencies installation into a singular command. This change is designed to make the first-time setup as straightforward as possible, aligning with our project's goal of being accessible and welcoming to new contributors.

Rationale:
The motivation behind these changes is to simplify the contribution process, making it more appealing and less daunting for individuals looking to contribute to our project. By streamlining the setup instructions, we hope to encourage a broader range of contributions and foster a more inclusive community.

As an individual contributor to this open-source project, I noticed an opportunity to enhance the contributor experience by simplifying the initial setup instructions within our documentation. This commit aims to lower the entry barrier for new contributors by providing a concise, all-in-one command sequence that facilitates the setup process.

Key Updates:
- Introduced a command for cloning the repository, ensuring newcomers start off on the right foot without having to navigate through multiple setup steps.
- Integrated the PATH environment variable update (`~/.local/bin`) within the setup commands to eliminate the need for manual configuration, making the environment preparation seamless.
- Combined cloning, environment setup, and dependencies installation into a singular command. This change is designed to make the first-time setup as straightforward as possible, aligning with our project's goal of being accessible and welcoming to new contributors.

Rationale:
The motivation behind these changes is to simplify the contribution process, making it more appealing and less daunting for individuals looking to contribute to our project. By streamlining the setup instructions, we hope to encourage a broader range of contributions and foster a more inclusive community.

Signed-off-by: Victor Boyd <115438604+Victor-Boyd@users.noreply.github.com>

.. code-block:: console

git clone https://github.com/ros2/ros2_documentation.git
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to contribute, you should fork, then clone your fork. To be able to push, you need to use ssh cloning.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure if we want to introduce how to create pull request here? in general https://docs.ros.org/en/rolling/The-ROS2-Project/Contributing/Developer-Guide.html#change-control-process looks good enough for me? besides, that is the common process for any ROS 2 repositories.

here i believe that adding explanation where requirements.txt is located is better for user document. i think using mainline would be just fine as example build process here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, I don't think every open source repository needs to explain how to use Github and forks to contribute code.

Copy link
Collaborator

@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.

@Victor-Boyd thank you for the PR, i have a couple of comments.


.. code-block:: console

git clone https://github.com/ros2/ros2_documentation.git
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure if we want to introduce how to create pull request here? in general https://docs.ros.org/en/rolling/The-ROS2-Project/Contributing/Developer-Guide.html#change-control-process looks good enough for me? besides, that is the common process for any ROS 2 repositories.

here i believe that adding explanation where requirements.txt is located is better for user document. i think using mainline would be just fine as example build process here.

Comment on lines +53 to +54
export PATH="~/.local/bin:$PATH" && pip3 install --user --upgrade -r requirements.txt

Copy link
Collaborator

Choose a reason for hiding this comment

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

if we want to add export, other platform command lines should be updated accordingly?

Suggested change
export PATH="~/.local/bin:$PATH" && pip3 install --user --upgrade -r requirements.txt
export PATH="~/.local/bin:$PATH" && pip3 install --user --upgrade -r requirements.txt

Copy link
Contributor

Choose a reason for hiding this comment

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

On Ubuntu 22, it's already part of ~/.profile.
cat ~/.profile

# set PATH so it includes user's private bin if it exists
if [ -d "$HOME/.local/bin" ] ; then
    PATH="$HOME/.local/bin:$PATH"
fi

https://askubuntu.com/questions/1144231/home-local-bin-not-in-path-for-ubuntu-19-04

Here's a dockerfile to demonstrate:

FROM ubuntu:22.04
ARG DOCKER_USER=ryan
#RUN useradd -ms /bin/bash ${DOCKER_USER} -g root -G sudo

RUN apt-get update && apt-get install sudo
RUN adduser --disabled-password --gecos '' ${DOCKER_USER}
USER ${DOCKER_USER}
WORKDIR /home/${DOCKER_USER}
CMD ["whoami"]
docker build . -t docs
docker run -w /home/ryan --net host -it docs bash
cat ~/.profile

Copy link
Contributor

Choose a reason for hiding this comment

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

For what it is worth, I don't think it is a good idea for us to be suggesting people use export PATH=~/.local/bin:$PATH. That will almost inevitably lead to people filing bugs because we "broke" their system, and also will not work on Windows. Instead I'd much rather we say something like Use /path/to/sphinx, where sphinx might be in ~/.local/bin.

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.

None yet

4 participants