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

Issue #82 - Add commands without subsystem to default subsystem group #83

Merged
merged 1 commit into from
Sep 11, 2018

Conversation

MJJoyce
Copy link
Member

@MJJoyce MJJoyce commented Aug 23, 2018

Commands are added to a default subsystem group on the frontend
during command dictionary parsing so that components that expect
commands to be grouped as such function as expected. Specifically, this
allows the CommandSearch object to get commands grouped as expected for
display. Since the subsystem attribute isn't required this means that the
CommandSearch component will default to all commands showing up under
this default subsystem header instead of not at all.

Resolve #82

Commands are added to a default subsystem group on the frontend
during command dictionary parsing so that components that expect
commands to be grouped as such function as expected. Specifically, this
allows the CommandSearch object to get commands grouped as expected for
display. Since the subsystem attribute isn't required this means that the
CommandSearch component will default to all commands showing up under
this default subsystem header instead of not at all.
@MJJoyce MJJoyce requested review from a team as code owners August 23, 2018 23:39
@MJJoyce
Copy link
Member Author

MJJoyce commented Aug 23, 2018

Not sure how I feel about this PR even after pushing it. Not liking the idea of "modifying" the command dictionary to have a subsystem that doesn't exist. Might need to rethink it ...

On second thought, I'm ok with this. The original dictionary is intact and this update is making sure that our "by subsystem" split of the data has all of the commands present in it.

@MJJoyce MJJoyce merged commit 95d7f96 into master Sep 11, 2018
@MJJoyce MJJoyce deleted the issue-82 branch September 11, 2018 20:39
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.

1 participant