Skip to content

Conversation

@csukuangfj
Copy link
Contributor

No description provided.

csukuangfj and others added 2 commits December 4, 2017 22:52
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
[ament_tools](https://github.com/ament/ament_tools) is a Python package which provides the command line tool `ament` to build, test, install, and uninstall packages.
It is similar to [catkin_tools](https://github.com/catkin/catkin_tools) and builds each package in a workspace in topological order.
Support for different build systems is integrated through extension points which allows to contribute support for other build types without change the ament tool itself.
Support for different build systems is integrated through extension points which allows to contribute support for other build types without changing the ament tool itself.
Copy link
Contributor

Choose a reason for hiding this comment

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

as long as you're here, I think the words to contribute can be deleted as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Heh, I just changed that myself. Not sure if you just want to remove "to contribute", or if you like my "allows the implementation of". I'm fine either way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reads fine to me!

@dirk-thomas
Copy link
Member

"build types" are imo not "implemented". I think saying that a build type like cmake is "supported" is more precise. Therefore I would suggest:

which allows to support other build types without changing the ament tool itself.

@dirk-thomas
Copy link
Member

"build types" are imo not "implemented". I think saying that a build type like cmake is "supported" is more precise. Therefore I would suggest:

which allows to support other build types without changing the ament tool itself.

@sloretz @mikaelarguedas Do you have any comment on this?

@dhood
Copy link
Member

dhood commented Dec 5, 2017

I agree with you @dirk-thomas . Would just change the english to "Support for different build systems is integrated through extension points which allows support for other build types to be added without changing the ament tool itself."

@mikaelarguedas
Copy link
Member

mikaelarguedas Do you have any comment on this?

The current state of this PR seems fine to me:
"Support for different build systems is integrated through extension points which allows to contribute support for other build types without changing the ament tool itself."

@dhood'd suggestion is fine as well it just changes "which allows to contribute support for other build types" to "which allows support for other build types to be added".

I dont think it deserves to be changed but maybe it's more english.

@sloretz
Copy link
Contributor

sloretz commented Dec 5, 2017

I'm happy with either "supported" or "implemented".

@dirk-thomas
Copy link
Member

I updated it to:

... which allows support for other build types to be added without changing the ament tool itself.

I will leave it up to someone else to squash and merge this. 👍 from me.

@dirk-thomas
Copy link
Member

@csukuangfj Thank you for your attention to detail.

@sloretz sloretz merged commit 89ccbaf into ros2:gh-pages Dec 5, 2017
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.

6 participants