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.

Sign up for GitHub

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

Jump to bottom

add all platform for all robots #17

Merged
merged 2 commits into from
Apr 24, 2020
Merged

add all platform for all robots #17

merged 2 commits into from
Apr 24, 2020

Conversation

knorth55
Copy link
Contributor

add all for platform

@knorth55 knorth55 mentioned this pull request Jan 21, 2020
Merged
@v4hn
Copy link
Member

v4hn commented Feb 3, 2020

From the patch it looks like not specifying a platform at all might do the same as the proposed "all" tag. Do we really need that patch?

@knorth55
Copy link
Contributor Author

knorth55 commented Feb 5, 2020

not specifying platform is also for all robots.
but i think it is good to have platform: all too for readability of config file of apps.

v4hn
v4hn reviewed Feb 5, 2020
View reviewed changes
Copy link
Member

@v4hn v4hn left a comment

Choose a reason for hiding this comment

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

I'm ok with the idea to add all as a keyword.
Please look at the inline comment though.

I will leave the decision to merge this to @k-okada as the patch comes from your lab.

src/app_manager/app_list.py Outdated Show resolved Hide resolved
@v4hn v4hn merged commit 9577ac5 into PR2:kinetic-devel Apr 24, 2020
@v4hn
Copy link
Member

v4hn commented Apr 24, 2020

Thanks for all your minor fixes @knorth55 ! 👍

@knorth55 knorth55 deleted the add-all branch April 24, 2020 11:49
@knorth55 knorth55 mentioned this pull request Dec 1, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@v4hn v4hn v4hn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@knorth55 @v4hn