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.

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

Part #274, Cleanup and add doxygen for cfe_psp.h #381

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Feb 27, 2023

Checklist

Describe the contribution

I have not attempted to split up the header, nor removed any functions - I intended to leave those as separate work items in the original issue or open a new issue for them. Can also remove functions in this PR if there is consensus/advice to do so.

Doxygen markup added, along with more detail for @param's, @return's etc.

Moved some existing function header information from the implementation files into cfe_psp.h - unless there is platform-specific information in that particular variant's function header comment. The .h file is the logical and central 'source-of-truth' for the function header information, and doubling it up (or tripling/quadrupling in the case of PSP) just leads to inconsistencies when updates occur.

Comments referring to BSPReadCDSRtn, BSPWriteCDSRtn and PSPRestartRt were removed as these seem to be no longer implemented - unless they exist in some other non-public implementation?

Also, is PtrToDataToRead a bit of a misnomer? (the pointer is for storage of the data no?)

Testing performed
Just GitHub CI.

Expected behavior changes
Documentation changes only.

Contributor Info
Avi Weiss @thnkslprpt

@skliper skliper changed the title Fix #274, Cleanup and add doxygen for cfe_psp.h Part #274, Cleanup and add doxygen for cfe_psp.h Dec 11, 2023
@skliper
Copy link
Contributor

skliper commented Dec 11, 2023

Change Fix to Part in commit comment, rebase. Great technical debt reduction!

@thnkslprpt thnkslprpt changed the title Part #274, Cleanup and add doxygen for cfe_psp.h Fix #274, Cleanup and add doxygen for cfe_psp.h Dec 11, 2023
@thnkslprpt
Copy link
Contributor Author

Change Fix to Part in commit comment, rebase. Great technical debt reduction!

@skliper That's all done mate

@skliper skliper changed the title Fix #274, Cleanup and add doxygen for cfe_psp.h Part #274, Cleanup and add doxygen for cfe_psp.h Dec 12, 2023
@skliper
Copy link
Contributor

skliper commented Dec 12, 2023

@thnkslprpt commit still says Fix.... I think you can do a git commit --amend and change it to say Part #274...

@thnkslprpt thnkslprpt changed the title Part #274, Cleanup and add doxygen for cfe_psp.h Fix #274, Cleanup and add doxygen for cfe_psp.h Dec 12, 2023
@thnkslprpt thnkslprpt changed the title Fix #274, Cleanup and add doxygen for cfe_psp.h Part #274, Cleanup and add doxygen for cfe_psp.h Dec 12, 2023
@thnkslprpt
Copy link
Contributor Author

@thnkslprpt commit still says Fix.... I think you can do a git commit --amend and change it to say Part #274...

@skliper Sorry - got things backwards. Should be correct now.

@skliper
Copy link
Contributor

skliper commented Dec 13, 2023

Note I also took out the word "fixes" from the PR text so it won't link the issue for automatic closure. Good to go!

@skliper skliper added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Dec 13, 2023
@dmknutsen dmknutsen added CCB:Approved Indicates Approval by CCB and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Dec 14, 2023
@dmknutsen dmknutsen merged commit 49c3175 into nasa:main Dec 14, 2023
11 checks passed
@thnkslprpt thnkslprpt deleted the fix-274-cfe_psp.h-cleanup branch December 14, 2023 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates Approval by CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants