Fix #1780, RTEMS CFE_FT_Global build failure

[astrogeco]

Describe the contribution

• Fix RTEMS CFE_FT_Global build failure  #1780

Testing performed
cFE CI and bundle CI
See succesful bundle tests at https://github.com/astrogeco/cFS/runs/3284780164

Expected behavior changes
No behavior change, fixes build failure on RTEMS 4.11 and 5.0

System(s) tested on
GitHub Actions CI

Additional context
Fixed as part of IC:2021-08-10

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
@astrogeco
@nmullane

[astrogeco]

@nmullane here's the PR for our fix

[skliper]

Minor nit - commit message looks like it's missing a verb. Preference would be "Fix #1780, Resolve RTEMS CFE_FT_Global build failure ..."

[skliper]

If this is just used by cfe_test_table.c it's probably more appropriate to initialize there.

[astrogeco]

It's used by multiple functions across multiple source files. The alternative is to re-initialize in each one of them or initialize them in the first one but that introduces a test execution order dependency which feels too brittle.

[skliper]

In that case I'm good as-is.

[astrogeco]

Specifically

TBLContentAccessTestSetup();
TBLContentMangTestSetup();
TBLInformationTestSetup();
TBLRegistrationTestSetup();

An alternative would be to create a new function say TBLTests_Setup that only contains this initialization.

[jphickey]

I actually prefer the idea of making a "Setup" function here that does this init, then it can be passed as the setup function as part of the UtTest_Add for any test that uses this table struct. That being said, its OK as is, I wouldn't hold this up.

[nmullane]

I don't think doing it as part of setup function would work out because there is already a setup function which is used by most but not all of the table tests to register a table. Also since CFE_FT_Global is already instantiated in cfe_test.c I think it would make the most sense to also perform the initialization of this data just the once either there or perhaps it could be moved into cfe_test_table.c.

[astrogeco]

Opened an issue for future work #1797

[astrogeco]

Minor nit - commit message looks like it's missing a verb. Preference would be "Fix #1780, Resolve RTEMS CFE_FT_Global build failure ..."

Minor nit - commit message looks like it's missing a verb. Preference would be "Fix #1780, Resolve RTEMS CFE_FT_Global build failure ..."

Nit on the nit, wouldn't "Fix" be the verb in this case?

[skliper]

Minor nit - commit message looks like it's missing a verb. Preference would be "Fix #1780, Resolve RTEMS CFE_FT_Global build failure ..."

Minor nit - commit message looks like it's missing a verb. Preference would be "Fix #1780, Resolve RTEMS CFE_FT_Global build failure ..."

Nit on the nit, wouldn't "Fix" be the verb in this case?

Nope. You are "fixing" the #issue, by verbing the subject. To be hones even my suggested update isn't that great... much more helpful to state what you actually did, as in "Fix #1780, Removed multiple instantiations of CFE_FT_Global to resolve RTEMS build issue". Even better:

Fix #1780, Resolve RTEMS CFE_FT_Global build falure

 - Removed multiple instantiations of CFE_FT_Global
 - Moved initialization of CFE_FT_Global table info to main test routine
 - Swapped order of source files in build

[jphickey]

Not intending to start any tribal wars, but I tend to side with @astrogeco on this one - The "Fix" verb should be sufficient. That is, if you simply take out the issue number and said "Fix RTEMS CFE_FT_Global build failure" - that still makes sense. The #issue is just an additional descriptor in this case, to indicate what issue was fixed (and adhere to our pattern).

As space within the first line of an commit log is precious, I don't want to expend space on two verbs that basically mean the same thing ("Fix" and "Resolve"). Keep that for the long description part. In some ways its like saying "ATM Machine" (another pet peeve of mine).

That's my 2c at least - I don't really care either way, aside from the general requirement to keep the first line as short as possible (under 50 chars is ideal) so it works well with various tools/displays that use the first line of a commit message.

[jphickey]

I actually prefer the idea of making a "Setup" function here that does this init, then it can be passed as the setup function as part of the UtTest_Add for any test that uses this table struct. That being said, its OK as is, I wouldn't hold this up.

[skliper]

Not intending to start any tribal wars, but I tend to side with @astrogeco on this one - The "Fix" verb should be sufficient. That is, if you simply take out the issue number and said "Fix RTEMS CFE_FT_Global build failure" - that still makes sense. The #issue is just an additional descriptor in this case, to indicate what issue was fixed (and adhere to our pattern).

As space within the first line of an commit log is precious, I don't want to expend space on two verbs that basically mean the same thing ("Fix" and "Resolve"). Keep that for the long description part. In some ways its like saying "ATM Machine" (another pet peeve of mine).

That's my 2c at least - I don't really care either way, aside from the general requirement to keep the first line as short as possible (under 50 chars is ideal) so it works well with various tools/displays that use the first line of a commit message.

What I'm really stuck on is the commit is just restating the issue... I definitely agree saying "Fix #xxx, Fixed issue blah" (or any other synonym of "fix") isn't useful. I agree my initial suggestion doesn't solve that. What I'd much rather see is a summary of the changes.