Fix #288, Remove unnecessary CF_UnionArgs_Payload_t union

nasa / CF

The Core Flight System (cFS) CFDP application.

Apache License 2.0

76 stars 45 forks source link

Fix #288, Remove unnecessary CF_UnionArgs_Payload_t union #398

Closed dzbaker closed 1 year ago

dzbaker commented 1 year ago

Checklist

[x] I reviewed the Contributing Guide.
[x] I signed and emailed the appropriate Contributor License Agreement to GSFC-SoftwareRelease@mail.nasa.gov and copied cfs-program@lists.nasa.gov.

Describe the contribution Fixes #288 CF_UnionArgs_Payload_t has been removed, given that only a single member of the 3 is used in CF. That member variable - byte - has been moved into the CF_UnionArgsCmd_t struct, which was the only place where CF_UnionArgs_Payload_t was used. Re-introduced from #341. #393, #395 reverted this PR.

Testing performed GitHub CI actions (incl. Build + Run, Unit Tests etc.) all passing successfully.

Expected behavior changes No impact on logic.

Contributor Info @thnkslprpt

dzbaker commented 1 year ago

@thnkslprpt I reverted the original (#341) so we could move it to the next release. Does everything look good here?

thnkslprpt commented 1 year ago

@thnkslprpt I reverted the original (#341) so we could move it to the next release. Does everything look good here?

@dzbaker I'm unsure of the effects of this PR as it includes a few other merged commits on top of the original commit removing the CF_UnionArgs_Payload_t union.

It's quite a simple change - maybe easier if I submit a new PR just going from the current main branch with just the removal of CF_UnionArgs_Payload_t?

dzbaker commented 1 year ago

@thnkslprpt I reverted the original (#341) so we could move it to the next release. Does everything look good here?

@dzbaker I'm unsure of the effects of this PR as it includes a few other merged commits on top of the original commit removing the CF_UnionArgs_Payload_t union.

It's quite a simple change - maybe easier if I submit a new PR just going from the current main branch with just the removal of CF_UnionArgs_Payload_t?

@thnkslprpt Yes, thank you, it may be best to submit a new PR and I can close this one.

thnkslprpt commented 1 year ago

@thnkslprpt Yes, thank you, it may be best to submit a new PR and I can close this one.

Cool no worries. Done (https://github.com/nasa/CF/pull/400)

dzbaker commented 1 year ago

Replaced by #400.