Skip to content

Fix #117 #155, Command handling updates#272

Merged
dzbaker merged 1 commit intonasa:mainfrom
havencarlson:Fix#117
Jul 14, 2022
Merged

Fix #117 #155, Command handling updates#272
dzbaker merged 1 commit intonasa:mainfrom
havencarlson:Fix#117

Conversation

@havencarlson
Copy link
Copy Markdown
Contributor

@havencarlson havencarlson commented Jul 6, 2022

Checklist (Please check before submitting)

Describe the contribution
A clear and concise description of what the contribution is.
Fix #117, an information event message is sent if a command is successful
Fix #155, Remove CF_CmdCon()

Testing performed
Ran unit tests

Expected behavior changes

  • Behavior Change: All commands now send an information event message if successful

System(s) tested on

  • OS: Ubuntu 18.04

Contributor Info - All information REQUIRED for consideration of pull request
Haven Carlson - NASA

@havencarlson havencarlson requested a review from skliper July 6, 2022 15:34
Copy link
Copy Markdown

@github-advanced-security github-advanced-security AI left a comment

Choose a reason for hiding this comment

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

Found 24 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link
Copy Markdown

@github-advanced-security github-advanced-security AI left a comment

Choose a reason for hiding this comment

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

Found 24 potential problems in the proposed changes. Check the Files changed tab for more details.

@havencarlson havencarlson changed the title Fix#117, validated commands send an info event message Fix #117, validated commands send an info event message Jul 6, 2022
Copy link
Copy Markdown

@skliper skliper left a comment

Choose a reason for hiding this comment

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

Great work! It looks to me like this would actually make fixing #118 easier right? Basically just add an event for each of the rejection cases.

@skliper
Copy link
Copy Markdown

skliper commented Jul 6, 2022

I think if you change the commits to "Fix #117, ..." they should pass.

@havencarlson havencarlson added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Jul 6, 2022
Copy link
Copy Markdown

@github-advanced-security github-advanced-security AI left a comment

Choose a reason for hiding this comment

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

Found 24 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link
Copy Markdown

@github-advanced-security github-advanced-security AI left a comment

Choose a reason for hiding this comment

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

Found 24 potential problems in the proposed changes. Check the Files changed tab for more details.

@havencarlson havencarlson marked this pull request as ready for review July 6, 2022 19:57
@havencarlson havencarlson force-pushed the Fix#117 branch 2 times, most recently from 4d9aea6 to dca163b Compare July 12, 2022 18:36
@dzbaker
Copy link
Copy Markdown
Contributor

dzbaker commented Jul 13, 2022

CCB 7/13/2022: Actions: Remove CmdConds and create ticket for Accept and Reject.

@dzbaker dzbaker added CCB:Approved and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Jul 13, 2022
@skliper
Copy link
Copy Markdown

skliper commented Jul 13, 2022

Recommend updating title and commit message to something like "Fix #117 #155, validated commands send an info event message and remove CF_CmdCond"... or you could break it into a short and long description like:

Fix #117 #155, Command handling event updates

- #117: Validated commands send info event message
- #155: Remove CF_CmdCond

@havencarlson havencarlson changed the title Fix #117, validated commands send an info event message Fix #117 #155, Command handling updates Jul 13, 2022
- nasa#117: Validated commands send info event message
- nasa#155: Remove CF_CmdCond
@dzbaker dzbaker merged commit c023d56 into nasa:main Jul 14, 2022
@skliper skliper added this to the Draco milestone Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implementation of CF_CmdCond takes address of inline function Not all validated commands send an info event message

5 participants