Skip to content

Commit

Permalink
Bump to v7.0.0-rc4+dev87
Browse files Browse the repository at this point in the history 
IC: Caelum-rc4+dev6, nasa/cFS#443

- Reverts development version identifier to 99 for revision number
  • Loading branch information
@astrogeco
astrogeco committed Mar 16, 2022
1 parent 848912c commit e5be061
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
6 changes: 6 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ The detailed cFE user's guide can be viewed at <https://github.com/nasa/cFS/blob

## Changelog

### Development Build: v7.0.0-rc4+dev87

- Revert development version identifier to 99 for revision number
- Use osal-common.doxygen to resolve OSAL Doxygen refs
- Refactor doxygen mainpage into frontpage
- See <https://github.com/nasa/cFE/pull/2066> and <https://github.com/nasa/cFS/pull/443>
### Development Build: v7.0.0-rc4+dev80

- Missing SB include for v2 msgid
Expand Down
2 changes: 1 addition & 1 deletion docs/src/cfs_versions.dox
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
bug fixes or major documentation updates.
The Revision number may also be updated if there are other changes contained within a release that make
it desirable for applications to distinguish one release from another.
WARNING: The revision number is set to the number 99 in development builds. To distinguish between development
WARNING: The revision number is set to the number 0xFF in development builds. To distinguish between development

This comment has been minimized.

Sign in to view

Copy link
@skliper

skliper Mar 19, 2022

Contributor

I strongly recommend against changing this via an unreviewed mechanism... especially < two weeks before a code drop.

This comment has been minimized.

Sign in to view

Copy link
@jphickey

jphickey Mar 23, 2022

Contributor

FWIW, I know I'm late to this discussion, but I thought we had gone through this several times before. We actually have 4 numbers defined to express a version - major, minor, rev, and "mission rev".

If my memory serves correctly, the last one (mission rev) was the one that the framework NEVER sets, because it is intended for users to indicate their own customization level, on top of the baseline release. It was this field that was supposed to be set as 0xFF in dev releases, because of several reasons:

  1. It allows a build to be flagged as "development" without disturbing the original major.minor.rev tuple (i.e. one can have a dev build based on e.g. v6.8.0 and v6.8.1)
  2. Missions should generally not be making changes to dev builds, unless they are contributing to the dev (or put another way - development is an "exclusive or" with that field's other purpose) so there is little chance of contention/confusion
  3. Even if (2) doesn't hold true, it would take 254 revisions before contention occurred, so risk is very low.

This comment has been minimized.

Sign in to view

Copy link
@skliper

skliper Mar 23, 2022

Contributor

For this drop (scheduled for Friday) we need to stick with historical convention of setting the rev to 99 for development builds. We can debate changing things (again) after that if you want.

This comment has been minimized.

Sign in to view

Copy link
@jphickey

jphickey Mar 23, 2022

Contributor

Point is, I thought this WAS what was agreed to all along, from the previous cycle (bootes days). We haven't changed this recently. 0xFF in "mission rev" was the flag that simply indicates a dev build, and the major/minor/rev tuple is whatever it needs to be.

NOTE: I'm not necessarily saying do not set rev to 99 - these things are not exclusive of one another. I think setting rev to 99 is valid to indicate that this is a pre-release of the next minor (or major) rev bump, but we haven't done the rev bump yet. I'm perfectly OK with setting the revision to 99, but keep the mission rev defined as it was from back when we last went through this in Bootes time.

This comment has been minimized.

Sign in to view

Copy link
@jphickey

jphickey Mar 23, 2022

Contributor

To clarify, I'm advocating for NOT changing things right now, its just I guess we differ on what "not changing things" means here - my contention is that what was in the header file was what was agreed to for the last couple cycles.

Again, what I'm referring to has nothing to do with setting rev to 99 - if you want to do that, that's perfectly fine - but the dev/official build flag should remain as what was agreed to (mission rev being 0 = official, 0xFF = dev, anything else = mission-specific/user-customized/not-official).

This comment has been minimized.

Sign in to view

Copy link
@skliper

skliper Mar 23, 2022

Contributor

Whatever we may have agreed to change in the past it was not implemented/propagated across all the repos consistently and it's too late now to change for this drop since there's way too much inertia to change course in the last 3 days. Rev 99 indicates development version for this release. We can change it Monday if you want.

This comment has been minimized.

Sign in to view

Copy link
@skliper

skliper Mar 23, 2022

Contributor

Note, there were 2 repos that used mission rev 0xFF (which didn't align with the documentation)... Theres something like 18 that did follow the rev 99 documentation. TBH I don't care either what mission rev is set to, but we have always said this is for projects to set and I think leaving it as zero avoids any potential conflicts.

This comment has been minimized.

Sign in to view

Copy link
@jphickey

jphickey Mar 23, 2022

Contributor

Again, I have no objection to setting rev to 99, that's fine - but I thought "MISSION_REV" was set to 0xFF in most repos already. I was seeing it set back to 0 in this change, which is what I didn't like.

(checking now, seems CI/TO LAB had it set to 0, everything else was correctly set as 0xFF).

I recall there is some code somewhere that generates the event strings rather than using hardcoded versions, and did this by checking if "mission rev" was 0xFF. So setting it back to 0 will incorrectly cause that code to report itself as official, which would be wrong.

That's why I'd strongly encourage one to not set this back to 0 in haste. Feel free to set rev to 99, but all I ask is to not change mission_rev from the definition that was already in there for the past year or more...

This comment has been minimized.

Sign in to view

Copy link
@skliper

skliper Mar 23, 2022

Contributor

Well, then that code is broken when used with the GSFC apps. It was a mistake to build in that dependency. We don't "own" the mission rev, projects do. We should not build in dependencies in open source that relies on that number being anything. Either way it's broken this round, and was not implemented fully/consistently. I think it's a mistake to build logic around a number we don't control. Breaking that "mistake" that is broken already isn't going to make things worse in my opinion... it's already a mess.

This comment has been minimized.

Sign in to view

Copy link
@astrogeco

astrogeco Mar 23, 2022

Author Contributor

I'm sorry for the confusion I caused with these changes. The first time I saw the 0xFF was in nasa/osal#824. I remember I then updated the other repos to match but missed the documentation. I don't think we had an in-depth discussion on using 0xFF though.

This comment has been minimized.

Sign in to view

Copy link
@jphickey

jphickey Mar 23, 2022

Contributor

That's OK. And I recognize that the GSFC apps never got the updates (this makes sense, as this was discussed during Bootes, only the framework would reflect the pattern).

Correct that we don't own the mission rev numeric value, but we do as far as defining what it means and the range values it can have. It was quite some time ago, but I thought we had settled the definition being software will be officially released with the value always set to "0" (because this was historically the way things were delivered), and that the range of 1-254 could be used by missions to indicate whatever they want. At the time, it was chosen to reserve the value 0xFF/255 as a dev build flag, because this was unlikely to trigger any contention, and didn't require overloading the other fields for this, which is more likely to be contentious.

So yeah, please do set revision to 99 if that makes things better for the current code drop, all I'm asking is that we do not revert the definition of MISSION_REV as it has stood for many months now, some folks have started to actually use that definition as it was written.

This comment has been minimized.

Sign in to view

Copy link
@skliper

skliper Mar 23, 2022

Contributor

We've also definitely must absolutely definitively start referring to a single truth for defining what version numbers mean. We continue to get out of sync based on multiple definitions. I vote for cfsversions as defined in cfs_versions.dox as truth.
builds refer to the BUILD_NUMBER and BUILD_BASELINE detailed in the section "Identifying Development Builds".

The Mission Version number is set to zero in all official releases, and is reserved for the mission use.
Expand Down
8 changes: 4 additions & 4 deletions modules/core_api/fsw/inc/cfe_version.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@
/* Version Macro Definitions updated for official releases only */
#define CFE_MAJOR_VERSION 6 /**< @brief Major release version (Former for Revision == 99) */

This comment has been minimized.

Sign in to view

Copy link
@skliper

skliper Mar 19, 2022

Contributor

Change above just made this wrong (although I recommend sticking with 99)
#define CFE_MINOR_VERSION 7 /**< @brief Minor release version (Former for Revision == 99) */

This comment has been minimized.

Sign in to view

Copy link
@skliper

skliper Mar 19, 2022

Contributor

Change above made this wrong (although I recommend sticking with 99)
#define CFE_REVISION 0 /*!< @brief ONLY APPLY for OFFICIAL releases. Revision number. */
#define CFE_REVISION \
99 /*!< @brief * Set to 0 on OFFICIAL releases, and set to 99 on development versions. Revision number. */

This comment has been minimized.

Sign in to view

Copy link
@skliper

skliper Mar 19, 2022 •
edited
Loading

Contributor

N 0 on OFFICIAL release. I like that it's 99, but inconsistent with the change in the versioning doc.

This comment has been minimized.

Sign in to view

Copy link
@jphickey

jphickey Mar 23, 2022

Contributor

I don't see why revision cannot be nonzero on an official release ... In the past we have definitely done point releases (6.1.1, 6.2.2, 6.3.1, 6.4.1, 6.4.2, etc). Although the current mindset is seems to be to not do any hotfix/bugfix release and to just move on to the next dev cycle, that wasn't always the case, and probably won't stay the case....

/*!
* @brief Mission revision.
*
* Set to 0 on OFFICIAL releases, and set to 255 (0xFF) on development versions.
* Values 1-254 are reserved for mission use to denote patches/customizations as needed.
* Reserved for mission use to denote patches/customizations as needed.
*/
#define CFE_MISSION_REV 0xFF
#define CFE_MISSION_REV 0

This comment has been minimized.

Sign in to view

Copy link
@jphickey

jphickey Mar 23, 2022

Contributor

I think this was correct originally (see my previous comment)

#define CFE_STR_HELPER(x) #x /**< @brief Convert argument to string */
#define CFE_STR(x) CFE_STR_HELPER(x) /**< @brief Expand macro before conversion */
Expand Down

0 comments on commit e5be061

Please sign in to comment.