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

color: moving color args out of build verb and into main program args #158

Merged
merged 1 commit into from
Mar 16, 2015

Conversation

jbohren
Copy link
Contributor

@jbohren jbohren commented Mar 11, 2015

There are now a bunch of subcommands in catkin_tools which generate color, so this moves the actual processing of these arguments into the main catkin command. Note that the arguments still exist in the catkin build verb, but they are hidden.

@@ -145,6 +136,10 @@ def prepare_arguments(parser):
add('--no-summarize', '--no-summary', action='store_false', dest='summarize',
help='explicitly disable the end of build summary')

# Deprecated args now handled by sysargs
Copy link
Member

Choose a reason for hiding this comment

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

"not handled by catkin command"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'll change that

@wjwwood
Copy link
Member

wjwwood commented Mar 12, 2015

Other than the comment, +1

@NikolausDemmel
Copy link
Member

Does catkin make use of --no-color during the alias expansion output? On the current master it does not.

@NikolausDemmel
Copy link
Member

I actually just tested this and this PR appears to achieve that.

@jbohren
Copy link
Contributor Author

jbohren commented Mar 16, 2015

@wjwwood Updated the requested comment change.

wjwwood added a commit that referenced this pull request Mar 16, 2015
color: moving color args out of build verb and into main program args
@wjwwood wjwwood merged commit 887a64d into catkin:master Mar 16, 2015
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