Skip to content

Fix #1474, #1552, Resolve API prototype/implementation discrepancies#1551

Merged
astrogeco merged 3 commits intonasa:integration-candidatefrom
skliper:fix1474_prototype-scrub
May 25, 2021
Merged

Fix #1474, #1552, Resolve API prototype/implementation discrepancies#1551
astrogeco merged 3 commits intonasa:integration-candidatefrom
skliper:fix1474_prototype-scrub

Conversation

@skliper
Copy link
Contributor

@skliper skliper commented May 19, 2021 •
edited
Loading

Describe the contribution
Fix #1474 - This checks prototypes against implementation and fixes any differences (return type, parameter names, etc). For the most part implementation was considered truth (fixed prototype) except for CFE_Status_t return type change. Also fixes some but not all use of CFE_Status_t in the implementations.

Fix #1552 - note completed ut-stub updates after CCB, added in since they are trival.

Follow on work needed - Internal use of CFE_Status_t (#921)

Testing performed
Built and ran unit tests, all passed. Built usersguide and confirmed no errors.

Expected behavior changes
None

System(s) tested on

  • Hardware: Docker on laptop
  • OS: Ubuntu 18.04
  • Versions: Bundle main + these commits

Additional context
Builds on commit from @zachar1a submitted in #1531, now closed as duplicate to this PR

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC
Zachary Gonzalez - Individual

@skliper skliper added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 19, 2021
@skliper skliper added this to the 7.0.0 milestone May 19, 2021
This was referenced May 19, 2021
Closed
Closed
@astrogeco
Copy link
Contributor

astrogeco commented May 19, 2021 •
edited
Loading

CCB:2021-05-19 APPROVED

  • Builds on community contribution
  • Removes old UT_TimeHandler
  • There's open issues for future work

@astrogeco astrogeco added the CCB:Approved Indicates code review and approval by community CCB label May 19, 2021
zachar1a added 2 commits May 19, 2021 18:46
@zachar1a
Partial nasa#1474, changing return type from int32 to CFE_Status_t
7fa73a5 
Changing the implementation return types to CFE_Status_t
to match the function prototypes.
@zachar1a
Partial nasa#1474, Resolved API function/prototype mismatches
9c28130
@skliper skliper force-pushed the fix1474_prototype-scrub branch from 6006b8d to 7e3effd Compare May 19, 2021 19:27
@skliper skliper changed the title Partial #1474, Resolve API prototype/implementation discrepancies Fix #1474, Resolve API prototype/implementation discrepancies May 19, 2021
@skliper
Copy link
Contributor Author

skliper commented May 19, 2021

Added in the ut-stub updates with 7e3effd

@skliper skliper requested a review from jphickey May 19, 2021 19:35
jphickey
jphickey approved these changes May 19, 2021
View reviewed changes
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Note, going forward we should consider the names in the header/documentation to be part of the API. That is, a name change in the header is treated similarly to an API change like adding or removing an argument. That's because unit test hooks may now be referencing parameters by the names in the header/documentation.

So if/when in the future there are discrepancies found between header and implementation, it would be less impact (compatibility-wise) to update the implementation to match the header, rather than the header to match the implementation.

@astrogeco astrogeco changed the base branch from main to integration-candidate May 24, 2021 17:16
@astrogeco astrogeco added the community Community contribution, YAY! label May 24, 2021
@astrogeco
Copy link
Contributor

astrogeco commented May 24, 2021

@skliper I see that the commits are all partials. Does their combination result in a full fix for both #1474 and #1552?

@astrogeco astrogeco removed the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label May 24, 2021
@zachar1a
Copy link
Contributor

zachar1a commented May 25, 2021

@astrogeco I only changed the implementation to match the prototypes for #1474. No work was done on my part to complete #921, another issue was also mentioned to remove the extern from both prototype and implementation. I didn't know if that should be another ticket or go in with this, so I just left them there.

@skliper
Copy link
Contributor Author

skliper commented May 25, 2021

@astrogeco The full set of commits does fix those issues as described and linked in the PR.

@skliper skliper force-pushed the fix1474_prototype-scrub branch from 7e3effd to da3f996 Compare May 25, 2021 12:35
@skliper
Copy link
Contributor Author

skliper commented May 25, 2021

@astrogeco Amended the commit message on the last one since it alone fixed #1552, the first two together fixed #1474.

@astrogeco astrogeco changed the title Fix #1474, Resolve API prototype/implementation discrepancies Fix #1474, #1552, Resolve API prototype/implementation discrepancies May 25, 2021
@astrogeco astrogeco merged commit 077bd82 into nasa:integration-candidate May 25, 2021
@astrogeco astrogeco mentioned this pull request May 25, 2021
Merged
astrogeco added a commit to nasa/cFS that referenced this pull request May 25, 2021
@astrogeco
IC:2021-05-25, Add cfe PRs
a70d61c 
nasa/cFE#1551, fixes discrepancies (return type, parameter names, etc) between function protoypes and implementation. Updates stubs accordingly
@astrogeco astrogeco mentioned this pull request May 26, 2021
Merged
astrogeco added a commit to nasa/cFS that referenced this pull request May 26, 2021
@astrogeco
IC:2021-05-25, Combine cfe and osal
365e7d7 
Combines:

- cfe v6.8.0-rc1+dev593 (nasa/cFE#1568)
- osal v5.1.0-rc1+dev458 (nasa/osal#1050)

Includes:

- nasa/cFE#1524, add printf format casts
- nasa/cFE#1520, accept "NULL" as entry point
- nasa/cfe #1549, add capability to generate multiple tables
- nasa/cFE#1551, fixes discrepancies (return type, parameter names, etc) between function protoypes and implementation. Updates stubs accordingly

- nasa/osal#1026, Add count sem timeout test
- nasa/osal#1026, defer cancellation when BSP locked
@skliper skliper deleted the fix1474_prototype-scrub branch October 22, 2021 19:25
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

No one assigned

Labels

CCB:Approved Indicates code review and approval by community CCB community Community contribution, YAY!

Projects

None yet

Milestone

Caelum

Development

Successfully merging this pull request may close these issues.

Update ut-stubs related to prototype/implementation scrub, #1474/#1551 API Prototype/implementation mismatch scrub

4 participants

@skliper @astrogeco @zachar1a @jphickey