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

Fix #78, Align use of Table/Tbl mnemonics #191

Closed
wants to merge 1 commit into from

Conversation

thnkslprpt
Copy link
Contributor

Checklist

Describe the contribution
Fixes #78
Aligns use of Table/Tbl mnemonics in sample_app.
The macros and function parameters that used 'table' have been converted to 'tbl'.
I left the table struct (SAMPLE_APP_Table_t) as is for now, because this is the more common convention in cFS.

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

Expected behavior changes
No impact on behavior.

Contributor Info
Avi Weiss @thnkslprpt

@@ -443,13 +443,13 @@
/* Output CRC */
/* */
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
void SAMPLE_APP_GetCrc(const char *TableName)
void SAMPLE_APP_GetCrc(const char *TblName)

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion

All functions of more than 10 lines should have at least one assertion.
@dzbaker
Copy link
Contributor

dzbaker commented Nov 3, 2022

CCB 3 November 2022: Preference would be to spell out "table" in all cases rather than spell it "tbl" (according to coding standards). Closing for now.

@dzbaker dzbaker closed this Nov 3, 2022
@thnkslprpt
Copy link
Contributor Author

CCB 3 November 2022: Preference would be to spell out "table" in all cases rather than spell it "tbl" (according to coding standards). Closing for now.

Just to clarify @dzbaker - was the preference now for the all use of 'tbl' to be converted to 'table' (and 'TBL' to 'TABLE') in sample_app, or to close that issue without any action?

@thnkslprpt thnkslprpt deleted the fix-78-align-tbl-mnemonics branch November 3, 2022 20:59
@dzbaker
Copy link
Contributor

dzbaker commented Nov 4, 2022

CCB 3 November 2022: Preference would be to spell out "table" in all cases rather than spell it "tbl" (according to coding standards). Closing for now.

Just to clarify @dzbaker - was the preference now for the all use of 'tbl' to be converted to 'table' (and 'TBL' to 'TABLE') in sample_app, or to close that issue without any action?

Hi @thnkslprpt - Yes, the preferable format would ideally be 'TABLE' (typically, we'll spell out anything that is 6 or fewer characters). We discussed this at yesterday's CCB. Ideally, we would like to make them all consistent, but there's the risk that we'll miss something that will inadvertently result in something breaking somewhere, so we'd like to tackle formatting issues like these when we have more time/resources to fully test the changes.

@thnkslprpt
Copy link
Contributor Author

CCB 3 November 2022: Preference would be to spell out "table" in all cases rather than spell it "tbl" (according to coding standards). Closing for now.

Just to clarify @dzbaker - was the preference now for the all use of 'tbl' to be converted to 'table' (and 'TBL' to 'TABLE') in sample_app, or to close that issue without any action?

Hi @thnkslprpt - Yes, the preferable format would ideally be 'TABLE' (typically, we'll spell out anything that is 6 or fewer characters). We discussed this at yesterday's CCB. Ideally, we would like to make them all consistent, but there's the risk that we'll miss something that will inadvertently result in something breaking somewhere, so we'd like to tackle formatting issues like these when we have more time/resources to fully test the changes.

Cool no worries.
Thanks for the clarification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent use of TBL vs TABLE
2 participants