Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

FEEDBACK WANTED: Crafting better changelog fragments #64

Closed
samccann opened this issue Jan 18, 2022 · 18 comments
Closed

FEEDBACK WANTED: Crafting better changelog fragments #64

samccann opened this issue Jan 18, 2022 · 18 comments

Comments

@samccann
Copy link

Summary

Today, Ansible has minimal guidance on how a developer can craft an informative changelog fragment - https://docs.ansible.com/ansible/latest/community/development_process.html#creating-a-changelog-fragment

Changelogs should inform the end-users about the high-level influence on their workflows UX-wise that has happened since the previous release. These fragments should be written in a common style so the autogenerated results flow well together. Perhaps choosing past tense.

The following changlog fragment types are also added to the autogenerated Ansible porting guides and should have clear instructions on how to write these:

  • breaking changes
  • major changes
  • deprecated_features
  • removed_features

Additional Information

No response

@webknjaz
Copy link
Member

Sweet, thanks for starting this topic! Are you planning to link the materials from the earlier conversations?

@samccann
Copy link
Author

samccann commented Jan 18, 2022

This comment will be updated regularly based on the feedback received in this issue or other discussions.

Today, Ansible provides:

  • Porting guide (target audience, end users of Ansible) - The Ansible package generates this based on changelog fragments that have been tagged appropriately. ansible-core manually creates porting guide entries.

  • Changelogs (Target audience, end users AND developers) - Both Ansible package and ansible-core provide changelogs, but we don’t have a lot of guidance on what should be included in a ‘good’ changelog entry.

  • git commits (target audience - other developers)- available only in git.

Proposal for crafting better changelog fragments

Changelogs are predominantly for end users of Ansible or a collection. As such, add details about the change as if you were a user of Ansible. All changelog fragments should include a link to the issue tracker that the PR resolves. Use RST formatting as described in the Ansible Style guide.

> possibly we should list out common markup instead of pointing to the whole guide. URLs and module/plugin names etc are easier to read if marked up properly for example

each item below will have an example of a well-crafted fragment, once we decide on what is well-crafted :-)

Use the following guidelines to maintain consistency across projects and collections.

breaking_changes

MUST include changes that break existing playbooks or roles. This includes any change to existing behavior that forces users to update tasks. Breaking changes means the user MUST make a change when they update. Breaking changes MUST only happen in a major release of the collection. Displayed in both the changelogs and the Porting Guides. Write in present tense and clearly describe the new behavior that the end user must now follow.

core example
ansible-test - automatic installation of requirements for "cloud" test plugins no longer occurs. The affected test plugins are ``aws``, ``azure``, ``cs``, ``hcloud``, ``nios``, ``opennebula``, ``openshift`` and ``vcenter``. Collections should instead use one of the supported integration test requirements files, such as the ``tests/integration/requirements.txt`` file (https://github.com/ansible/ansible/pull/75605).

collection example
ec2_instance - instance wait for state behavior has changed. If plays require the old behavior of waiting for the instance monitoring status to become OK when launching a new instance, the action will need to specify ``state: started`` (https://github.com/ansible-collections/amazon.aws/pull/481).

major_changes

Major changes to ansible-core or a collection. SHOULD not include individual module or plugin changes. MUST include non-breaking changes that impact all or most of a collection (for example, updates to support a new SDK version across the collection). Major changes mean the user can CHOOSE to make a change when they update but do not have to. Could be used to announce an important upcoming EOL or breaking change in a future release. (ideally 6 months in advance, if known. See https://github.com/ansible-collections/community.general/blob/stable-1/CHANGELOG.rst#v1313 for an example). Displayed in both the changelogs and the Porting Guides. Write in present tense and describe what is new. Optionally, include a 'Previously..." sentence to help the user identify where old behavior should now change.

core example
ansible-test - all “cloud” plugins which use containers can now be used with all POSIX and Windows hosts. Previously the plugins did not work with Windows at all, and support for hosts created with the --remote option was inconsistent (https://github.com/ansible/ansible/pull/74216).

collection example
bitbucket_* modules - client_id is no longer marked as ``no_log=true``. If you relied on its value not showing up in logs and output, mark the whole tasks with ``no_log: true`` (https://github.com/ansible-collections/community.general/pull/2045).

minor_changes

Minor changes to ansible-core, modules, or plugins. This includes new parameters added to modules, or non-breaking behavior changes to existing parameters, such as adding additional values to choices[]. Minor changes are enhancements, not bug fixes. Write in present tense.

core example
lineinfile - add warning when using an empty regexp (https://github.com/ansible/ansible/issues/29443).

collection example
nmcli - adds ``routes6`` and ``route_metric6`` parameters for supporting IPv6 routes (https://github.com/ansible-collections/community.general/issues/4059).

deprecated_features

Features that have been deprecated and are scheduled for removal in a future release. Displayed in both the changelogs and the Porting Guides. Use past tense and include an alternative, where available for what is being deprecated.

core example
``include action`` - is deprecated in favor of ``include_tasks``, ``import_tasks`` and ``import_playbook`` (https://github.com/ansible/ansible/pull/71262).

collection example
mail callback plugin - not specifying ``sender`` is deprecated and will be disallowed in ``community.general`` 6.0.0 (https://github.com/ansible-collections/community.general/pull/4140).

removed_features

Features that were previously deprecated and are now removed. Displayed in both the changelogs and the Porting Guides. Use past tense and include an alternative, where available for what is being deprecated.

core example
``_get_item()`` alias - removed from callback plugin base class which had been deprecated in favor of ``_get_item_label()`` (https://github.com/ansible/ansible/pull/70233).

collection example
acme_account_facts - the deprecated redirect has been removed. Use ``community.crypto.acme_account_info`` instead (https://github.com/ansible-collections/community.crypto/pull/290).

security_fixes

Fixes that address CVEs or resolve security concerns. MUST use security_fixes for any CVEs. Include links to CVE information. Use present tense.

core example
``set_options`` -do not include params in exception when a call to ``set_options`` fails. Additionally, block the exception that is returned from being displayed to stdout. (CVE-2021-3620).

collection example
win_psexec - ensure password is masked in ``psexec_``command return result (https://github.com/ansible-collections/community.windows/issues/43).

bugfixes

Fixes that resolve issues. SHOULD not be used for minor enhancements (use minor_change instead). Use past tense to describe the problem and present tense to describe the fix.

core example
``ansible_play_batch`` - variable included unreachable hosts. Fix now saves unreachable hosts between plays by adding them to the PlayIterator's ``_play._removed_hosts`` (https://github.com/ansible/ansible/issues/66945).

collection example
apt_repository - fix crash caused by ``cache.update()`` raising an ``IOError`` due to a timeout in ``apt update`` (https://github.com/ansible/ansible/issues/51995).

known_issues

Known issues that are currently not fixed or will not be fixed. Use present tense and where available, use imperative tense for a workaround.

core example
ansible-test - tab completion anywhere other than the end of the command with the new composite options provides incorrect results (https://github.com/kislyuk/argcomplete/issues/351).

collection example
idrac_user - module may error out with the message unable to perform the import or export operation because there are pending attribute changes or a configuration job is in progress. Wait for the job to complete and run the task again.(https://github.com/dell/dellemc-openmanage-ansible-modules/pull/303)

@samccann
Copy link
Author

samccann commented Jan 18, 2022

Prior discussion - https://hackmd.io/ZdymdeUSQNeM0hMjsn7vig which includes links to some related topics on other community opensource projects. Also borrowed heavily from this - https://redhat-documentation.github.io/supplementary-style-guide/#release-notes

@samccann
Copy link
Author

I'm still hunting down any opensource guidelines I can find so if you know of others, pop a link in the comments please!

@mariolenz
Copy link
Contributor

major_changes

Major changes to Ansible itself.

Ansible or ansible-core? That is: Can a collection have major changes or not?

Generally does not include module or plugin changes.

Generally is generally (ha!) problematic. What are the exceptions?

Maybe it would be clearer to define that changes to a single module or plugin are never major. I would see a change to a module util as major because this would (possibly) affect several modules at once. Maybe this would be a good definition, but I'm not sure.

Thoughts?

@mariolenz
Copy link
Contributor

I'm still hunting down any opensource guidelines I can find so if you know of others, pop a link in the comments please!

I can't provide any helpful opensource guidelines at the moment, but I like RfC 2119 which defines the key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL".

Maybe it would be a good idea to refer to this RfC and use those keywords as defined there. Then you could define for example:

breaking_changes

Changes that break existing playbooks or roles MUST be documented in breaking_changes. This includes any change to existing behavior that forces users to update tasks. Displayed in both the changelogs and the Porting Guides. Write in present tense and clearly describe the new behavior that the end user must now follow.

Just an idea.

@samccann
Copy link
Author

@mariolenz agreed on your suggestions. We MUST :-) be clear on what has to be, what should be, etc. And I can add both a core and a collection example to each to help. I believe collections have used major changes as well but it was brought up in the community WG meeting today that it is a bit unclear when to use it so we'll need to be more specific for sure.

@jillr
Copy link

jillr commented Jan 19, 2022

samcann beat me to it. :) We've been using major_version in a number of collections for things that are notable at a collection level but generally not breaking.

My best example offhand walks the line of breaking and major but as a discussion point:

In (amazon|community).aws we've used major_changes on major collection revisions to denote SDK support for that release (we have a stated SDK support policy that increments on major collection release). This should generally not be breaking, but we do stop testing older SDK versions and cease guaranteeing support.

@Andersson007
Copy link
Contributor

I would change the following a bit in the minor_changes description:
.. or behavior changes to existing parameters -> .. or non- breaking behavior changes to existing parameters like new acceptable values (what we have in choices: [ ..., ... ]).
or something with similar meaning

@mariolenz
Copy link
Contributor

Is an issue the right place to discuss this? I suggest to use discussions in ansible-collections/news-for-maintainers. Or, alternatively, enable discussions in this repo.

Don't get me wrong, I'm fine with discussing better changelog fragments here. I just think that there might be better places to do it.

@webknjaz
Copy link
Member

webknjaz commented Feb 6, 2022

ansible/community#384

@samccann
Copy link
Author

samccann commented Feb 8, 2022

I've updated the comment above to put in examples and the feedback so far. The formatting isn't perfect and I still need to dig up some github issues to go with each, but please review and see if you think these are good examples or not.

@mariolenz - I'm actually not sure when a community topic should be in the new news discussions. @felixfontein do you have ideas on when we should use these topics here vs the discussions in the collection news repo?

@felixfontein
Copy link
Contributor

news-for-maintainers is definitely the wrong repository to discuss this; this is the right one. Whether we should use GH issues or GH discussions is probably a matter of taste. Both have advantages and disadvantages. I guess for now we should stay in this issue and discuss here.

@mariolenz
Copy link
Contributor

news-for-maintainers is definitely the wrong repository to discuss this; this is the right one.

At the moment I'm a little bit overwhelmed by the many points of contact and places to discuss things. I'm not always sure where a question belongs. Thanks for clarifying this @felixfontein!

Whether we should use GH issues or GH discussions is probably a matter of taste. Both have advantages and disadvantages. I guess for now we should stay in this issue and discuss here.

Sure, no problem. I just wanted to raise the question if this is the right place. But it looks like it is :-)

@samccann
Copy link
Author

samccann commented Feb 9, 2022

Documenting some feedback from the community meeting:

  • update examples to use our current fragment formatting guidelines
  • Clarify breaking changes mean I MUST do something (change a playbook etc) whereas major changes mean I can CHOOSE to do something different but I don't have to.
  • Clarify that Breaking changes MUST happen only in a major release.
  • use MUST/SHOULD etc as per RFC reference in prior comments.
  • Remove 'new features' from the list of examples that should be a minor change.
  • Clarify how a changelog could be used to announce an important upcoming EOL or breaking change in a future release. (ideally 6 months in advance, if known). These are individual fragments (typically major_change) not tied to a code change in the current release that the changelog appears, for example https://github.com/ansible-collections/community.general/blob/stable-1/CHANGELOG.rst#v1313

@samccann samccann added the being_implemented This is currently being implemented label Feb 14, 2022
@samccann
Copy link
Author

At this point, this discussion is now in the implementation phase and I'll link a PR here to the docs changes for final review.

@samccann
Copy link
Author

Please post comments and reviews on the PR that implements this - ansible/ansible#77040

@samccann
Copy link
Author

Resloved by ansible/ansible#77040 Thanks everyone!

@samccann samccann added implemented and removed being_implemented This is currently being implemented labels Feb 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants