Skip to content

Fix #522 - SocketOpen stub should set sock_id#523

Merged
astrogeco merged 1 commit intonasa:integration-candidatefrom
CDKnightNASA:fix-522-socketopen_stub
Jul 1, 2020
Merged

Fix #522 - SocketOpen stub should set sock_id#523
astrogeco merged 1 commit intonasa:integration-candidatefrom
CDKnightNASA:fix-522-socketopen_stub

Conversation

@CDKnightNASA
Copy link
Contributor

@CDKnightNASA CDKnightNASA commented Jun 23, 2020 •
edited by astrogeco
Loading

Describe the contribution
Fix #522 - OS_SocketOpen() should, on "success", set sock_id and return a status.

Additional context
See OS_SocketOpen() definition in

osal/src/os/inc/osapi-os-net.h

Line 232 in bc73437

* @param[out] sock_id Buffer to hold the non-zero OSAL ID

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

@CDKnightNASA CDKnightNASA added the unit-test Tickets related to the OSAL unit testing (functional and/or coverage) label Jun 23, 2020
@CDKnightNASA CDKnightNASA requested a review from jphickey June 23, 2020 14:59
@CDKnightNASA CDKnightNASA self-assigned this Jun 23, 2020
@CDKnightNASA CDKnightNASA added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Jun 23, 2020
@CDKnightNASA CDKnightNASA marked this pull request as ready for review June 23, 2020 17:54
@skliper
Copy link
Contributor

skliper commented Jun 23, 2020

Missing explicit keywords to autolink with issue. Either use them (Fix #xxx) or link by hand to issue.

@CDKnightNASA
Copy link
Contributor Author

CDKnightNASA commented Jun 23, 2020

Missing explicit keywords to autolink with issue. Either use them (Fix #xxx) or link by hand to issue.

Weird, if you look at the text of the PR, it has an autolink (hover over #522 above). Why it doesn't link it under linked issues is beyond me. :/

@CDKnightNASA CDKnightNASA linked an issue Jun 23, 2020 that may be closed by this pull request
OS_SocketOpen UT stub returns "socket ID" not status #522
Closed
@jphickey
Copy link
Contributor

jphickey commented Jun 24, 2020

Weird, if you look at the text of the PR, it has an autolink (hover over #522 above). Why it doesn't link it under linked issues is beyond me. :/

A simple text reference like "#522" will result in a clickable hyperlink but github does not assume that this PR will fix/close that issue unless you actually say "Fixes #522" or one of the other specific closing keywords. (not "Fix for").

See https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

@astrogeco astrogeco added CCB-20200624 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jun 24, 2020
@astrogeco
Copy link
Contributor

astrogeco commented Jun 24, 2020

CCB-20200624: Approved

@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB IC-20200624 labels Jul 1, 2020
@astrogeco astrogeco changed the base branch from master to integration-candidate July 1, 2020 13:50
@astrogeco astrogeco changed the title fix #522 - SocketOpen stub should set sock_id Fix #522 - SocketOpen stub should set sock_id Jul 1, 2020
@astrogeco astrogeco merged commit 57701da into nasa:integration-candidate Jul 1, 2020
@astrogeco astrogeco mentioned this pull request Jul 1, 2020
Merged
@skliper skliper added this to the 5.1.0 milestone Aug 21, 2020
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
Fix nasa#523, SB Subscription report control on separate MID
f211a06
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
@astrogeco
Merge pull request nasa#647 from skliper/fix523-sb-sub-rpt-mid
159ab75 
Fix nasa#523, SB Subscription report control on separate MID
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
@CDKnightNASA
missed a MID wrap, other fixes related to ticket nasa#666, nasa#523, n…
070af1e 
…asa#580
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jphickey jphickey jphickey approved these changes

Assignees

@CDKnightNASA CDKnightNASA

Labels

CCB:Approved Indicates code review and approval by community CCB unit-test Tickets related to the OSAL unit testing (functional and/or coverage)

Projects

None yet

Milestone

Bootes

Development

Successfully merging this pull request may close these issues.

OS_SocketOpen UT stub returns "socket ID" not status

4 participants

@CDKnightNASA @skliper @jphickey @astrogeco