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

Rename files, functions, and variables to match APPNAME_* pattern #77

Closed
astrogeco opened this issue Jun 17, 2020 · 4 comments · Fixed by #100 or #102
Closed

Rename files, functions, and variables to match APPNAME_* pattern #77

astrogeco opened this issue Jun 17, 2020 · 4 comments · Fixed by #100 or #102
Assignees
Labels
breaking bug Something isn't working
Milestone

Comments

@astrogeco
Copy link
Contributor

Describe the bug
PR #73 shed light on inconsistencies in naming conventions. There are multiple items named SAMPLE_itemname as opposed to SAMPLE_APP_itemname.

To Reproduce
One relevant example is SAMPLE_TblValidationFunc in sample_app.c:226

Expected behavior
All item names should use the prefix SAMPLE_APP as opposed to SAMPLE. In the example above the correct name should then be SAMPLE_APP_TblValidationFunc.

System observed on:
Source Code

Reporter Info
Gerardo E. Cruz-Ortiz, NASA-GSFC

@astrogeco astrogeco added the bug Something isn't working label Jun 17, 2020
@jphickey
Copy link
Contributor

jphickey commented Jun 19, 2020

I would also like to suggest that as part of renaming any files - also check that they reside in the appropriate directory.

In particular - the sample_app_msg.h, sample_app_events.h, and sample_app_table.h should reside in the mission_inc directory as these are mission-scope definitions that other entities must use or reference - namely the ground system. These really shouldn't be stuck in the src directory which should only be used for private, internal-use header files.

As far as I can tell, the only header file in the src directory that actually belongs there is sample_app_version.h.

@astrogeco
Copy link
Contributor Author

Agreed, also whenever we implement this we should use the git mv command

@jphickey
Copy link
Contributor

I mention it because at least one user ran into trouble and emailed me because they ran into trouble when including the message definitions from their app's "src" directory externally in other apps - when I pointed out that external/public API like message defintions should be in a separate "inc" directory - turns out the only reason they had it in src was because that's where sample_app had it.

So I really think its worth the investment to fix all the naming/file/directory patterns and make sure they really are the model we want users to adhere to. Including for the upcoming bundle release associated with cfe 6.8.

@jphickey
Copy link
Contributor

Actually running into issues due to this name inconsistency - probably worth fixing this....

@jphickey jphickey self-assigned this Oct 27, 2020
@skliper skliper added this to the 1.3.0 milestone Oct 27, 2020
jphickey added a commit to jphickey/sample_app that referenced this issue Oct 27, 2020
Replace inconsistent SAMPLE_ and SAMPLE_App name prefixes,
now all identifers should start with SAMPLE_APP_.
jphickey added a commit to jphickey/sample_app that referenced this issue Oct 27, 2020
Replace inconsistent SAMPLE_ and SAMPLE_App name prefixes,
now all identifers should start with SAMPLE_APP_.
jphickey added a commit to jphickey/sample_app that referenced this issue Oct 30, 2020
Replace inconsistent SAMPLE_ and SAMPLE_App name prefixes,
now all identifers should start with SAMPLE_APP_.
astrogeco added a commit that referenced this issue Nov 2, 2020
Fix #77, Standardize to SAMPLE_APP_ namespace prefix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking bug Something isn't working
Projects
None yet
3 participants