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

Fix #392, Make OS_STREAM_STATE available with APIs #393

Closed
wants to merge 1 commit into from
Closed

Fix #392, Make OS_STREAM_STATE available with APIs #393

wants to merge 1 commit into from

Conversation

CDKnightNASA
Copy link
Contributor

@CDKnightNASA CDKnightNASA commented Mar 26, 2020 •
edited
Loading

…it's needed by OS_SelectSingle()

Describe the contribution
moves OS_STREAM_STATE defines to os/inc/osapi-os-core.h as they're needed by OS_SelectSingle()

Testing performed
Can build unit tests (being developed) for OS_SelectSingle()

Note that this is a prerequisite for #377 and is related to #391 all three should be pulled at the same time, or #392, #391 then #377

Contributor Info - All information REQUIRED for consideration of pull request
[email protected]

skliper
skliper requested changes Mar 26, 2020
View reviewed changes
Copy link
Contributor

@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.

Recommend rebase when the API scrub pull request gets merged, and then add doxygen comments to document the states/typedef per the API scrub implemented pattern.

@skliper skliper added the bug label Mar 26, 2020
@skliper skliper added this to the 5.1.0 milestone Mar 26, 2020
@CDKnightNASA CDKnightNASA mentioned this pull request Mar 26, 2020
Closed
@CDKnightNASA
Copy link
Contributor Author

Recommend rebase when the API scrub pull request gets merged, and then add doxygen comments to document the states/typedef per the API scrub implemented pattern.

Confused, is there a separate ticket/pull request for an API scrub? If so, have a ticket #?

@skliper
Copy link
Contributor

skliper commented Mar 26, 2020

It's in the current integration candidate #375, was the fix to #364... pull request was #371.

I think just pending @astrogeco merge of #375.

@skliper
Copy link
Contributor

skliper commented Mar 27, 2020

@CDKnightNASA recommend a rebase now that #375 is merged, and just add comments that follow the current pattern. Or I can add them if you want.

@skliper skliper changed the title fix for #392 - moves OS_STREAM_STATE... to os/inc/osapi-os-core.h as … Fix #392, Make OS_STREAM_STATE available with APIs Mar 27, 2020
@CDKnightNASA CDKnightNASA force-pushed the fix-392-os_stream_state_defines branch from 9849a4a to f0fae6a Compare March 27, 2020 16:36
@CDKnightNASA
Copy link
Contributor Author

rebased, commented, and squashed

skliper
skliper reviewed Mar 27, 2020
View reviewed changes
src/os/inc/osapi-os-core.h Outdated Show resolved Hide resolved
@CDKnightNASA CDKnightNASA force-pushed the fix-392-os_stream_state_defines branch from f0fae6a to 1cca69f Compare March 27, 2020 17:42
@astrogeco astrogeco added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Mar 31, 2020
@astrogeco
Copy link
Contributor

CCB 2020-04-01 - APPROVED

@astrogeco astrogeco added CCB - 20200401 CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Apr 1, 2020
@astrogeco astrogeco linked an issue Apr 1, 2020 that may be closed by this pull request
OS_SelectSingle(fd, &SelectFlags, timeout) users require OS_STREAM_STATE_READABLE/OS_STREAM_STATE_WRITABLE #392
Closed
@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Approved Indicates code review and approval by community CCB labels Apr 1, 2020
@skliper
Copy link
Contributor

skliper commented Apr 3, 2020

@CDKnightNASA - why close? They close when the update gets to master...

@skliper
Copy link
Contributor

skliper commented Apr 3, 2020

Whoops, I mean when the pull request gets to the IC branch

@CDKnightNASA
Copy link
Contributor Author

CDKnightNASA commented Apr 3, 2020 •
edited
Loading

Closing the pull request, not the issue. Will create a new pull request that incorporates the 3 related tickets, plus going to wait for #405

@skliper
Copy link
Contributor

skliper commented Apr 3, 2020

Ohhhh, perfect! My mistake. When I first clicked I thought it was the issue.

jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
@skliper
20191106/13: Merge nasa#387 nasa#390 nasa#391 nasa#393 nasa#395 nasa#400
9aedaab 


Fixes nasa#379, Fixes nasa#380, Fixes nasa#383, Fixes nasa#384,
Fixes nasa#385, Fixes nasa#392
Code reviewed and approved at 20191106 and 20191113 CB
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skliper skliper skliper approved these changes

Assignees
No one assigned
Labels
bug CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Milestone
Bootes
Development

Successfully merging this pull request may close these issues.

OS_SelectSingle(fd, &SelectFlags, timeout) users require OS_STREAM_STATE_READABLE/OS_STREAM_STATE_WRITABLE
3 participants
@CDKnightNASA @skliper @astrogeco