Skip to content

Add detailed guide for building ros2_rust packages #129

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

Merged
merged 10 commits into from
May 19, 2022
Merged

Conversation

nnmm
Copy link
Contributor

@nnmm nnmm commented Apr 27, 2022

Also extend contributing.md and condense the readme a bit. See here for a rendered version.

@nnmm nnmm requested review from esteve and jhdcs April 27, 2022 19:40
Copy link
Collaborator

@jhdcs jhdcs left a comment

Choose a reason for hiding this comment

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

Only a partial review at the moment, I'll have to look at the rest later...

Copy link
Collaborator

@jhdcs jhdcs left a comment

Choose a reason for hiding this comment

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

Just a few more suggested changes. That being said, I like how this is turning out!

Copy link
Collaborator

@jhdcs jhdcs left a comment

Choose a reason for hiding this comment

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

I apologize for not catching this one earlier...

jhdcs
jhdcs previously approved these changes May 4, 2022
@nnmm
Copy link
Contributor Author

nnmm commented May 4, 2022

Thanks @jhdcs! Any more changes requested from your side, @esteve?

jhdcs
jhdcs previously approved these changes May 16, 2022
@esteve
Copy link
Collaborator

esteve commented May 17, 2022

@nnmm the Contributing.md file should go back to the root and renamed back to CONTRIBUTING.md. GitHub uses that convention so that contributors get a notification before they submit code (see https://github.blog/2012-09-17-contributing-guidelines/). This is in accordance with other projects not specific to ROS2 as well (e.g. https://github.com/django/django/blob/main/CONTRIBUTING.rst, https://github.com/pocoproject/poco/blob/master/CONTRIBUTING.md)

@nnmm
Copy link
Contributor Author

nnmm commented May 17, 2022

the Contributing.md file should go back to the root and renamed back to CONTRIBUTING.md. GitHub uses that convention so that contributors get a notification before they submit code (see https://github.blog/2012-09-17-contributing-guidelines/). This is in accordance with other projects not specific to ROS2 as well (e.g. https://github.com/django/django/blob/main/CONTRIBUTING.rst, https://github.com/pocoproject/poco/blob/master/CONTRIBUTING.md)

@esteve if you look up a newer article, GitHub also checks the docs subdirectory. I'd also be surprised if it needs to be uppercase, but I renamed it just to make sure.

@esteve
Copy link
Collaborator

esteve commented May 17, 2022

@nnmm you're right about the docs/ folder (https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors), but in that article seems to always refer to the file as CONTRIBUTING(.md), so perhaps it needs to be in uppercase anyway.

@nnmm
Copy link
Contributor Author

nnmm commented May 17, 2022

Yep, it's uppercase now.

@nnmm
Copy link
Contributor Author

nnmm commented May 19, 2022

@esteve Do you have any further comments?

@esteve
Copy link
Collaborator

esteve commented May 19, 2022

@nnmm that was all, thanks for addressing my feedback 🙂

@nnmm nnmm merged commit 6b57344 into master May 19, 2022
@nnmm nnmm deleted the doc_articles branch May 19, 2022 22:00
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