Allow to update deprecations/removals #177
Conversation
felixfontein
force-pushed
the
cancel-deprecation
branch
from
296d593 to
52be54f
Compare
| if version is None: | ||
| # The following line should never be reached: | ||
| pass # pragma: no cover | ||
| elif version.major != self.major_release: |
There was a problem hiding this comment.
So this means that if a collection was removed in Ansible X.A.0 and then re-added in Ansible X.B.0, we would just remove its removal: metadata when we copy over the collection meta file to the X+1 directory for the next major version?
There was a problem hiding this comment.
No, we'd keep the removal metadata, but copy it back from removed_collections to collections and add updates: and move announce_version to removed_version there, and re-add major_version. https://github.com/ansible-community/ansible-build-data/pull/488/files#diff-de27baf579d836d439eb1aa79999d825ae044f35799bb2e89b598bde6791a047 shows how it looks like.
There was a problem hiding this comment.
For the netapp.storagegrid example where the collection was re-added, I'm asking about what would happen when we create the 12 directory. We would just remove its removal: metadata because it would no longer apply to Ansible 12, right?
There was a problem hiding this comment.
Yes. If it later happens to be deprecated again in Ansible 12, a new removal entry can be added.
| @@ -44,6 +44,44 @@ def _convert_pypi_version(v: t.Any) -> t.Any: | |||
| PydanticPypiVersion = Annotated[PypiVer, BeforeValidator(_convert_pypi_version)] | |||
|
|
|||
|
|
|||
| class RemovalUpdate(p.BaseModel): | |||
There was a problem hiding this comment.
Should updates be able to have their own reasons? For example, if we are cancelling a deprecation, wouldn't we want the changelog entry for the removal update to reflect why we did that?
There was a problem hiding this comment.
That's a good point. I thought about it last weekend but didn't have time to implement it then; I'll try to add it tomorrow.
I'm wondering a bit whether this should be free-text, or similar to the reason field in the top-level removal info. Then we'd likely also need a set of more fields (like there). I'll think about it when I have more time...
There was a problem hiding this comment.
I'll take a look at the new version of this when I get a chance. Thanks!
|
At some point, we should document this all in https://github.com/ansible-community/ansible-build-data/blob/main/docs/policies.md#removal-from-ansible |
Co-authored-by: Maxwell G <[email protected]>
|
Merging for now as discussed on Matrix. |
This is needed to generate a consistent changelog if https://forum.ansible.com/t/2811/24 (or similar votes) succeed.