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

SC has significant blocks of #ifdef code #105

Closed
3 tasks done
jphickey opened this issue Sep 21, 2023 · 0 comments · Fixed by #106
Closed
3 tasks done

SC has significant blocks of #ifdef code #105

jphickey opened this issue Sep 21, 2023 · 0 comments · Fixed by #106

Comments

@jphickey
Copy link
Contributor

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Code that is "compiled-out" is supposed to be avoided. However, SC has significant chunks of such code, particularly inside #if (SC_ENABLE_GROUP_COMMANDS == true) blocks.

Having blocks of disabled code makes testing difficult (it is only built one way, disabled code can't be tested) and is against current coding standards for FSW.

Ideally, functionality should be included all the time, if possible. But if the goal is to save code space necessitating code removal, then the preferred pattern is to isolate the optional implementation to completely separate source files, and have an alternate implementation of the same routines that simply return an error code that the function is not implemented.

Describe the solution you'd like
Remove #if and #ifdef preprocessor statements

Describe alternatives you've considered
Isolate functions to separate source file and do source selection in cmake

Requester Info
Joseph Hickey, Vantage Systems, Inc.
jphickey added a commit to jphickey/SC that referenced this issue Sep 22, 2023
@jphickey
Fix nasa#105, remove SC_ENABLE_GROUP_COMMANDS option
db6fb35 
This was always set true and the software has not been validated with
the option set to false.  Turning this off only removed a few small
functions and the complexity (and violation of coding standard) does not
justify it as an option.
@jphickey jphickey mentioned this issue Sep 22, 2023
Merged
2 tasks
jphickey added a commit to jphickey/SC that referenced this issue Sep 22, 2023
@jphickey
Fix nasa#105, remove SC_ENABLE_GROUP_COMMANDS option
2270da4 
This was always set true and the software has not been validated with
the option set to false.  Turning this off only removed a few small
functions and the complexity (and violation of coding standard) does not
justify it as an option.
jphickey added a commit to jphickey/SC that referenced this issue Sep 22, 2023
@jphickey
Fix nasa#105, remove SC_ENABLE_GROUP_COMMANDS option
016d971 
This was always set true and the software has not been validated with
the option set to false.  Turning this off only removed a few small
functions and the complexity (and violation of coding standard) does not
justify it as an option.
dzbaker added a commit that referenced this issue Sep 28, 2023
@dzbaker
Merge pull request #106 from jphickey/fix-105-remove-conditional-code
304c166 
Fix #105, remove SC_ENABLE_GROUP_COMMANDS option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #105, remove SC_ENABLE_GROUP_COMMANDS option jphickey/SC
1 participant
@jphickey