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 merge conflicts in PR #998 #1028

Closed
wants to merge 4 commits into from
Closed

Fix merge conflicts in PR #998 #1028

wants to merge 4 commits into from

Conversation

astrogeco
Copy link
Contributor

@astrogeco astrogeco commented Nov 23, 2020 •
edited
Loading

Describe the contribution
Took #998 and rebased on top of integration-candidate and attempted to solve merge conflicts

Testing performed
Built on docker container. Unit tests pass.

See https://github.com/astrogeco/cFS/actions for more tests

@skliper
Fix #777, Use MSG APIs - Unit tests
8514efa 
Updates the unit tests, unit test support, and
stubs to replace deprecated SB APIs with MSG APIs.
@skliper
Fix #777, Use MSG APIs - Docs
1da6b3c 
Update documentation to replace deprecated SB
APIs with MSG APIs
@skliper
Fix #777, Use MSG APIs - Core software
34e5510 
Update the core software from the deprecated SB
APIs to the MSG APIs.
@astrogeco astrogeco changed the base branch from main to integration-candidate November 23, 2020 07:17
@astrogeco
Copy link
Contributor Author

@jphickey and @skliper I tried to resolve the merge conflicts in #998 but I probably messed some things up.

Some of the size_t changes did not play well with CFE_MSG_Size_t. I think I fixed all of them but it might be good to double check that everything works as intended.

Feel free to force push to this branch and then we can either merge this one instead of #998 or force push to skliper/fix777-deprecate-sb

astrogeco added a commit to astrogeco/cFS that referenced this pull request Nov 23, 2020
@astrogeco
Try to add nasa/cFE#1028
5d11fb1
@jphickey
Merge pull request #998 into integration-candidate
325e9d2 
Fix #777, Use MSG APIs - Core software
Fix #777, Use MSG APIs - Docs
Fix #777, Use MSG APIs - Unit tests
@jphickey
Copy link
Contributor

Updated with my take - basically I did the same but as a merge (not rebase) and just resolved the conflicts, tested, etc. Mostly the same as what you had aside from some whitespace diffs and a few size_t/CFE_MSG_Size_t diffs.

If you concur we should be able to just push (i.e. fast forward) the integration candidate here - don't really need a separate PR, I think. It's just a merge.

skliper
skliper reviewed Nov 23, 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.

I didn't check every line, but looks fine to me.

@astrogeco astrogeco marked this pull request as ready for review November 23, 2020 21:16
astrogeco pushed a commit to astrogeco/cFE that referenced this pull request Nov 23, 2020
@jphickey @astrogeco
Merge pull request nasa#1028 from nasa/fix777-deprecate-sb
7b9db48 
Fix nasa#777, Use MSG APIs - Core software
Fix nasa#777, Use MSG APIs - Docs
Fix nasa#777, Use MSG APIs - Unit tests

See nasa#998 for more details
astrogeco added a commit to astrogeco/cFS that referenced this pull request Nov 23, 2020
@astrogeco
Add nasa/cFE#1028
199b2b3
@astrogeco astrogeco closed this Dec 1, 2020
@astrogeco astrogeco deleted the fix777-deprecate-sb branch December 1, 2020 03:07
@astrogeco astrogeco mentioned this pull request Dec 1, 2020
Merged
astrogeco added a commit to astrogeco/cFS that referenced this pull request Dec 2, 2020
@astrogeco
Try to add nasa/cFE#1028
b191aa4
astrogeco added a commit to astrogeco/cFS that referenced this pull request Dec 2, 2020
@astrogeco
Add nasa/cFE#1028
b5101ef
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skliper skliper skliper left review comments

Assignees
No one assigned
Labels
invalid
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@astrogeco @jphickey @skliper