fix #759 - deprecates GetLastSenderId()

[CDKnightNASA]

Describe the contribution
Fixes #759 - deprecates CFE_SB_GetLastSenderId() API.
Fixes #608

Testing performed
make with and without OMIT_DEPRECATED defined.

System(s) tested on
Debian 10.3

Contributor Info - All information REQUIRED for consideration of pull request
Christopher.D.Knight@nasa.gov

[jphickey]

Looks OK to me.

Two minor nitpicks though:

• The existing OMIT_DEPRECATED flags have a CFE prefix.
• Also if introducing a new deprecation we need to add it to the global_build_options.cmake file so it gets turned on with OMIT_DEPRECATED=true like the others do.

We might want to revisit the deprecation process WRT the constellation names, as it might make more sense to do e.g. OMIT_DEPRECATED_BOOTES instead of using CFE versions here.

[jphickey]

Another note - I believe for other APIs that have been deprecated, rather than clutter up the unit test code with #ifdef directives - we can just remove it outright, as (AFAIK) there is no requirement to unit test deprecated functions. UT code is ugly enough already.

[CDKnightNASA]

Looks OK to me.

Two minor nitpicks though:

• The existing OMIT_DEPRECATED flags have a CFE prefix.

Whoop, don't know how I missed this!

• Also if introducing a new deprecation we need to add it to the global_build_options.cmake file so it gets turned on with OMIT_DEPRECATED=true like the others do.

Copy.

I've squashed updates.

[CDKnightNASA]

Another note - I believe for other APIs that have been deprecated, rather than clutter up the unit test code with #ifdef directives - we can just remove it outright, as (AFAIK) there is no requirement to unit test deprecated functions. UT code is ugly enough already.

Done, the stubs remain (of course) but are likewise #ifndef'd.

[skliper]

Moved the requirement back to "submitted" in requirement tracking tool. Will need to export and update text requirements.

[CDKnightNASA]

Moved the requirement back to "submitted" in requirement tracking tool. Will need to export and update text requirements.

Would it be worthwhile to export the requirements into an integrated requirements tracking system, like https://pypi.org/project/doorstop/ ?

[astrogeco]

CCB 2020-07-29, APPROVED, @skliper to provide doxygen deprecation example.

[astrogeco]

@CDKnightNASA can you move the cmake 6_8 definition into it's own commit?

[skliper]

Example of deprecated doxygen markings:

cFE/fsw/cfe-core/src/inc/cfe_fs.h

Lines 194 to 214 in deeb294

	/*****************************************************************************/
	/**
	** \brief DEPRECATED; Determines if a file is a Gzip/compressed file.
	** \deprecated
	**
	** \par Description
	** This API will check the filename and return true if the file is
	** a gzip file. The check is currently based on the filename, so the
	** zipped files should use the ".gz" extention.
	**
	** \par Assumptions, External Events, and Notes:
	** -# A gzipped file will use the ".gz" filename extention.
	**
	** \param[in] FileName The name of the file.
	**
	** \return Boolean for file has ".gz" extension
	** \retval true File has ".gz" extension
	** \retval false File does not have ".gz" extension
	**
	******************************************************************************/
	bool CFE_FS_IsGzFile(const char *FileName);

[astrogexo]

Moved the requirement back to "submitted" in requirement tracking tool. Will need to export and update text requirements.

Would it be worthwhile to export the requirements into an integrated requirements tracking system, like https://pypi.org/project/doorstop/ ?

I looked into it and it looks interesting but I'm not sure what it buys us. Have you used it?

[skliper]

Would need to trade against the internal tools and process requirement compliance. We get some things for "free" using standard internal processes, but that doesn't mean it's better.

[CDKnightNASA]

Moved the requirement back to "submitted" in requirement tracking tool. Will need to export and update text requirements.

Would it be worthwhile to export the requirements into an integrated requirements tracking system, like https://pypi.org/project/doorstop/ ?

I looked into it and it looks interesting but I'm not sure what it buys us. Have you used it?

I have not, but some folks at the 2019 FSW mentioned they used it. Certainly would need to do a study of it.

[CDKnightNASA]

@CDKnightNASA can you move the cmake 6_8 definition into it's own commit?

done

[CDKnightNASA]

CCB 2020-07-29, APPROVED, @skliper to provide doxygen deprecation example.

done, ready for review again

[astrogeco]

@skliper this looks good to me, are we good to merge?

[skliper]

Can this go in the 6.8 dev cycle? It's wrapped with the 6.8 omit define. Other than that, yes looks good to me.

[skliper]

Recommend merge to 6.8 development

[astrogeco]

Recommend merge to 6.8 development

If you you mean make this part of 6.8 then yes!

[skliper]

No, I mean development on top of 6.8. Otherwise the OMIT numbering breaks convention. I'm requesting making the rc tags BEFORE merging this change.

[skliper]

Marked my preference as targeting this for 7.0 release. Sorry, my quick comments weren't very clear.

[astrogeco]

The milestone helps, thanks!

[astrogeco]

What's the convention then? Should the deprecate tag then say deprecate 7_0 or 7_1?

[skliper]

What's the convention then? Should the deprecate tag then say deprecate 7_0 or 7_1?

6_8.

[astrogeco]

What's the convention then? Should the deprecate tag then say deprecate 7_0 or 7_1?

6_8.

For completeness then, does DEPRECATE_6_8 means that 6.8 was the last version which supports this function?

[skliper]

We've typically just marked with the last released version, so it could be interpreted that way but I'm not aware of any hard rules or published deprecation process. See nasa/cFS#67.

[jphickey]

does DEPRECATE_6_8 means that 6.8 was the last version which supports this function?

Yes, that is the paradigm we've been using up to this point -- the number reflects the last release were it was not deprecated. The idea of putting a version number in the tag was so we'd know which ones had "aged out" in a future release and should be taken out completely.

However as I mentioned before now that we have a codename for API compatibility then maybe we should consider using that instead.

[astrogeco]

@CDKnightNASA does this actually fix #608? Just wondering since you don't mention it in the commits

[CDKnightNASA]

@CDKnightNASA does this actually fix #608? Just wondering since you don't mention it in the commits

Yes it fixes #608.