camaraproject / DeviceLocation

Repository to describe, develop, document and test the DeviceLocation API family
Apache License 2.0
20 stars 33 forks source link

AreaType as enum #132

Closed jlurien closed 6 months ago

jlurien commented 6 months ago

What type of PR is this?

What this PR does / why we need it:

Fix desalignment regarding areaType values

Which issue(s) this PR fixes:

Fixes #131

Special notes for reviewers:

To be incorporated to RC

Changelog input

* areaType is now an enum in UPPER_CASE for all 3 APIs
jlurien commented 6 months ago

@akoshunyadi please review also

jlurien commented 6 months ago

The "content" looks good to me. But I think we should change the versions to 0.1.0-wip/0.2.0-wip again, otherwise we will have multiple versions with -rc.

I don't know how to proceed here with the versioning. The fix has to be applied to main branch for sure and also to the RC. Now both branches were sync'd.

In case of renaming the versions in main, they should be "0.3.0-wip" and "0.2.0-wip" from now on, to reflect that they are the WIP versions for the following release, while the stable releases are in their dedicated branches. We have the option to fix the current -rc, which was very recent and not still announced officially, or create a new -rc2. Taking into account that we are just releasing this RC and the additional effort to create a new release with branch, changelog, etc, I'm in favour of fixing 0.10.0-rc and communicate that the RC is ready (with this fix). If we have more fixes in the coming weeks we may consider creating a new -rc2.

Wdyt @akoshunyadi , @bigludo7 ?

hdamker commented 6 months ago

Hi @jlurien @akoshunyadi we don't have yet an official way to handle these situation, therefore I can only share my thinking and how we have handled it in QoD previously:

a) a release candidate is not a release, so there is no need to have a permanent branch for it. But it's a tagged commit, there is the changelog update for it and it has the version number b) while the 0.n.0-rc is out but not yet released, my recommendation would be to merge to main only fixes which must go also into the release 0.n.0, and don't merge PRs which are already meant for the next release c) the version within the YAMLs to be changed back to "v0.2.0-wip" ... which actually is not a version but just a hint that this commit is part of the work in progress towards the release, and it's "unreleased". d) if after some fixes you decide that it would make sense to create another release candidate before the release then indeed v0.2.0-rc2 would be the right version for that (with changes between rc1 and rc2 documented in the changelog) e) the actual release v0.n.0 would then be done always based on main, with documenting the complete changes compared to the previous release again in the changelog

A different approach would be to continue in main already with 0.n+1.0-wip and use the rc branch to cherry-pick the needed fixes. My impression is that this is not supported directly in github but can done with GitHub desktop (see here) or git. The advantage would be that the work on the next release could already be startet ... but than can be confusing I suppose.

akoshunyadi commented 6 months ago

Isn't there an agreed recommendation for releasing already? + @hdamker

hdamker commented 6 months ago

Isn't there an agreed recommendation for releasing already? + @hdamker

There are Camara_Versioning_Guidelines.md but they don't say anything about release candidates yet and leave freedom to sub projects:

The commonalities WG does not impose any restrictions on the sub projects for creating other branches, tags which might be needed/useful to address their specific requirements. The above said guidelines are only applicable for the subproject releases.

There are several issues open regarding release management which need to be addressed, see for an overview https://github.com/camaraproject/ReleaseManagement/issues/9

jlurien commented 6 months ago

Thanks @hdamker for your input. some comments inline:

Hi @jlurien @akoshunyadi we don't have yet an official way to handle these situation, therefore I can only share my thinking and how we have handled it in QoD previously:

a) a release candidate is not a release, so there is no need to have a permanent branch for it. But it's a tagged commit, there is the changelog update for it and it has the version number

I agree with that, but here I was copying the model I saw in other subprojects, particualrly QoD, where we indeed have a dedicated branch for 0.10.0-rc.

b) while the 0.n.0-rc is out but not yet released, my recommendation would be to merge to main only fixes which must go also into the release 0.n.0, and don't merge PRs which are already meant for the next release

Totally agree here. In this case, it is a fix which should be taken forward to future releases, so it makes sense to fix main as well.

c) the version within the YAMLs to be changed back to "v0.2.0-wip" ... which actually is not a version but just a hint that this commit is part of the work in progress towards the release, and it's "unreleased".

That would work as long as we are not adding any new content intended for future release.

d) if after some fixes you decide that it would make sense to create another release candidate before the release then indeed v0.2.0-rc2 would be the right version for that (with changes between rc1 and rc2 documented in the changelog) e) the actual release v0.n.0 would then be done always based on main, with documenting the complete changes compared to the previous release again in the changelog

We may hold this fix just in main branch for a while, and create a new RC2 is some weeks, with this a other fixes to come. However, I find that having the current RC without the fix may confuse people as well. It's like saying, forget about that 0.2.0-rc and test the one in main. So exceptionally, and due to this fix being reported very fast I would cherrypick this fix into the current RC as well.

A different approach would be to continue in main already with 0.n+1.0-wip and use the rc branch to cherry-pick the needed fixes. My impression is that this is not supported directly in github but can done with GitHub desktop (see here) or git. The advantage would be that the work on the next release could already be startet ... but than can be confusing I suppose.

That would be necessary if RC period is so long that work on next release is suspended, but so far RC period are usually few weeks. We can avoid that by now, I think

hdamker commented 6 months ago

@jlurien also inline, as I have two comments (hope we don't need to continue that):

Thanks @hdamker for your input. some comments inline:

Hi @jlurien @akoshunyadi we don't have yet an official way to handle these situation, therefore I can only share my thinking and how we have handled it in QoD previously: a) a release candidate is not a release, so there is no need to have a permanent branch for it. But it's a tagged commit, there is the changelog update for it and it has the version number

I agree with that, but here I was copying the model I saw in other subprojects, particualrly QoD, where we indeed have a dedicated branch for 0.10.0-rc.

My bad ... yes, while the release-0.10.0-rc branch would not be necessary, I had created it to have consistent web links for the rc and the later actual release in the README.md.

b) while the 0.n.0-rc is out but not yet released, my recommendation would be to merge to main only fixes which must go also into the release 0.n.0, and don't merge PRs which are already meant for the next release

Totally agree here. In this case, it is a fix which should be taken forward to future releases, so it makes sense to fix main as well.

c) the version within the YAMLs to be changed back to "v0.2.0-wip" ... which actually is not a version but just a hint that this commit is part of the work in progress towards the release, and it's "unreleased".

That would work as long as we are not adding any new content intended for future release.

d) if after some fixes you decide that it would make sense to create another release candidate before the release then indeed v0.2.0-rc2 would be the right version for that (with changes between rc1 and rc2 documented in the changelog) e) the actual release v0.n.0 would then be done always based on main, with documenting the complete changes compared to the previous release again in the changelog

We may hold this fix just in main branch for a while, and create a new RC2 is some weeks, with this a other fixes to come. However, I find that having the current RC without the fix may confuse people as well. It's like saying, forget about that 0.2.0-rc and test the one in main. So exceptionally, and due to this fix being reported very fast I would cherrypick this fix into the current RC as well.

You can't change the 0.2.0-rc as it already published and probably used by some people (see also https://semver.org/#spec-item-3). But nothing is hindering you to immediately create a 0.2.0-rc2 with this fix. Or even create it within the fix PR if there is no other bug issue to be addressed before. Then also the question about the version within the YAML is solved (as it will be 0.2.0-rc2). Just add the new release candidate to the CHANGELOG.md and point within the README to the rc2 within the same PR.

jlurien commented 6 months ago

I have just versioned the fixed version as -rc2, so we can create a new release with the fix. Please review again @bigludo7, @akoshunyadi, @alpaycetin74

jlurien commented 6 months ago

Merging, as this is an important fix