Fix #888, Add typedef for function return status codes

[CDKnightNASA]

Closes #888

Describe the contribution
This adds a typedef for return status codes for functions. I've also added a brief comment to CFE_TBL_GetStatus as it's behavior will likely change in the future in the hopes of not having a non-zero "info" status.

Testing performed
make install

Expected behavior changes
no impact to behavior

System(s) tested on
Ubuntu 20.04

Contributor Info - All information REQUIRED for consideration of pull request
[email protected]

[astrogeco]

CCB 2020-09-30:

• Some behavior might warrant "re-architecteing",
• What about all the int32s scattered around the code? How do we help users transition to using the new typedef?
• to be "typesafe" we need to provide checks for each type (equality, etc.)
• code readability is one compelling driver for new types
• might want to change all the #defined macros related to cfe_error.h
• Some discussion about cFE vs OSAL return status codes
• @CDKnightNASA please link related tickets

APPROVED

[astrogeco]

@CDKnightNASA can you clear the conflicts?

[skliper]

I think the preference is rebase your branch on main, vs a merge of main into your branch... which complicates the concept of feature branches being based on main. My preference is conflict resolutions should go into the feature commit vs the main merge commit.

[jphickey]

Concur - this should be rebased onto main, not a merge. But since you already merged you should be able to fix it by checking out the branch and doing a git reset --soft main and then re-committing. That'll take your current content and turn it back into a normal commit based off main instead of a merge.

[CDKnightNASA]

@CDKnightNASA can you clear the conflicts?

I've done a reset and re-push, hopefully this is resolved? (I liked the github conflict resolution UI, too bad it does the resolution in a form y'all don't appreciate. :} )

[CDKnightNASA]

(Note this is an excellent reason to "phase" the typedef changes so we don't end up having to do a lot of conflict resolution in all of our sourcecode.)

[CDKnightNASA]

Gah, looks like my reset didn't fix the conflict, retrying...

[astrogeco]

@jphickey and @skliper could we use GitHub's rebase and merge or squash and merge button to "fix" the merge from main into feature branch? That way @CDKnightNASA can use the github conflict UI which is indeed pretty nice.

[astrogeco]

@CDKnightNASA looks like we have another conflict now that I changed the target to integration-candidate. The joys of collaboration!

[jphickey]

A change like this is bound to have lots of conflicts because these return codes are used on literally every function, so every change to any prototype will be a conflict.

As for merge conflict resolution, I use command lines with meld as my preferred (local) merge conflict resolution tool. My limited experience with the github GUI is that it doesn't offer sufficient control over the process so I don't even try.

My recommendation is (and always has been) to do things locally, test them, then push them. The biggest issue with github merges done via web interface is that it commits them immediately without giving you a chance to test/validate the result, which is an immediate fail/disqualification in my view.

[astrogeco]

A change like this is bound to have lots of conflicts because these return codes are used on literally every function, so every change to any prototype will be a conflict.

As for merge conflict resolution, I use command lines with meld as my preferred (local) merge conflict resolution tool. My limited experience with the github GUI is that it doesn't offer sufficient control over the process so I don't even try.

My recommendation is (and always has been) to do things locally, test them, then push them. The biggest issue with github merges done via web interface is that it commits them immediately without giving you a chance to test/validate the result, which is an immediate fail/disqualification in my view.

That's true, the conflict should resolve and get added to the branch for more testing not merged automatically. I also vouch for Meld. I've also had good experiences with some of Atom's git plugins.

[CDKnightNASA]

Process-wise, what I'm proposing is a schedule where each major change is developed, tested, merged separately...For example if we want to apply clang-format across the board, it would be good to schedule when that starts and when that ends and everyone should avoid making any other changes during that period. WRT typedefs, we need not be quite as strict but still have "exclusion periods" where one set of typedefs are updated while no work is done on other typedefs. A lot easier to avoid conflicts when these developments are performed completely serially.

[skliper]

My preference is to make format changes fairly atomic... warn everyone of the action, apply to the code base once all pending merges are in, quick review by a few key folks, merge to master, then notify everyone it's OK to start new work on it. It's easy with format since it is fast as long as we ignore everyone who still doesn't like the applied style. For other name global fast changes we can take a similar approach as long as the concept is approved in advance.

[skliper]

A change like this is bound to have lots of conflicts because these return codes are used on literally every function, so every change to any prototype will be a conflict.
As for merge conflict resolution, I use command lines with meld as my preferred (local) merge conflict resolution tool. My limited experience with the github GUI is that it doesn't offer sufficient control over the process so I don't even try.
My recommendation is (and always has been) to do things locally, test them, then push them. The biggest issue with github merges done via web interface is that it commits them immediately without giving you a chance to test/validate the result, which is an immediate fail/disqualification in my view.

That's true, the conflict should resolve and get added to the branch for more testing not merged automatically. I also vouch for Meld. I've also had good experiences with some of Atom's git plugins.

How about convert the PR to draft, do whatever you want to do using whatever tool you want even if it merges to the PR branch, pull the update and test it prior to converting back to non-draft? I don't follow why we'd need to be overly restrictive on the tool used as long as it's clear when a PR is ready or not.

[astrogeco]

A change like this is bound to have lots of conflicts because these return codes are used on literally every function, so every change to any prototype will be a conflict.
As for merge conflict resolution, I use command lines with meld as my preferred (local) merge conflict resolution tool. My limited experience with the github GUI is that it doesn't offer sufficient control over the process so I don't even try.
My recommendation is (and always has been) to do things locally, test them, then push them. The biggest issue with github merges done via web interface is that it commits them immediately without giving you a chance to test/validate the result, which is an immediate fail/disqualification in my view.

That's true, the conflict should resolve and get added to the branch for more testing not merged automatically. I also vouch for Meld. I've also had good experiences with some of Atom's git plugins.

How about convert the PR to draft, do whatever you want to do using whatever tool you want even if it merges to the PR branch, pull the update and test it prior to converting back to non-draft? I don't follow why we'd need to be overly restrictive on the tool used as long as it's clear when a PR is ready or not.

I think the problem was that GitHub's tool merges back to the target branch not the PR one. I'm all for people using their own tools and also sharing which ones work for them.

[jphickey]

Yes - you are free to use whatever tool you want.

The problem is immediately pushing a conflict-resolution merge to the IC that hasn't even been tested.

When you do the merge/rebase/whatever conflict resolution locally on your dev machine, you at least have the opportunity to validate that the result of your merge/rebase builds and runs before it gets pushed to the IC or where ever it is going. When you do it via Github UI it flat out skips that pesky "testing" step.

[skliper]

My point is there is nothing stopping you from doing the merge in the fix PR, pulling it and testing it. You still have the opportunity prior to pushing to an IC. If you do in GitHub or a local tool you can test it either way. There isn't just one way to do it as you seem to suggest.

EDIT - given what @astrogeco mentioned, you could likely do it in your fork or just do a rebase? Does it really force a merge to the target? That's bad.

EDITEDIT - wow, that is bad. Disregard everything I said (based assuming GitHub allowed a rebase without merge to target).