Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insert ansible-core version in the porting guide #397

Merged
merged 2 commits into from
Feb 9, 2022
Merged

Insert ansible-core version in the porting guide #397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f621e6
Insert ansible-core version in the porting guide.
felixfontein Feb 1, 2022
f15268f
Add changelog fragment.
felixfontein Feb 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/397-ansible-core-version-in-porting-guide.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- "Mention the ``ansible-core`` major version in the Ansible porting guide (https://github.com/ansible-community/antsibull/pull/397)."
6 changes: 6 additions & 0 deletions src/antsibull/build_changelog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,12 @@ def _get_porting_guide_bytes(changelog: Changelog) -> bytes:

# Determine whether to include ansible-core porting guide or not
if changelog.ansible_version.major == 2 or base_version != prev_base_version:
if changelog.ansible_version.major > 2:
builder.add_raw_rst(
# noqa: E501
"\n"
f"Ansible {version} is based on Ansible-core {base_version}."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on, or includes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh, a wordsmithing question from gundalow!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same formulation that we used in the past. "includes" is the language which we use in other places, though some folks don't like that (https://groups.google.com/g/ansible-project/c/XKOuAhD6pk0/m/9fCLXFiTBAAJ) and technically they are right. It depends on whether you look at it from POV of Python packaging, or from POV of an end-user.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that comment in google groups, I'm not sure 'based on' would be considered correct. Perhaps it is "Ansible foo also installs ansible-core bar"?

If that's a bit too much word bikeshedding, I'm quite happy to have based on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to decide at some point what we mean by the "Ansible (PyPi) package". If we mean the set of community collections, with a dependency on ansible-core, then we should throw out all ansible-core specific parts of the docs, including the ansible.builtin plugin documentation, from the Ansible docsite (and just have it in the ansible-core docsite). This is basically what that comment in google groups is asking for. Then we should write something like "depends on". If we on the other hand consider Ansible a big package which includes ansible-core, then we should use "based on" or "included". This is what we decided to do so far, and also what the docsite describes (by including the ansible-core docs in it).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs are most usable when they reflect the user experience. If I install Ansible (the PyPI package), I get ansible-core plus the set of community collections, right? And I don't have to choose a version or even confirm a version of ansible-core? If that's correct, then I'd still vote for "includes" here, because that matches what the user experiences. Yes, it can be overridden. But that takes an extra step. As a user, I'd rather have one set of docs that's correctly versioned for what I have installed, instead of having to confirm which version of ansible-core I have installed, then finding the right version of the docs for ansible-core after I've found the right version of ansible and its docs.

We can certainly add information about installing other versions of ansible-core. We can even add a sentence or two after each mention of Ansible "including" ansible-core that clarifies the mechanism and the fact that it can be overridden ("When you pip install ansible the requirements.txt file automatically installs the associated version of ansible-core, but the ansible-core package is built and distributed separately on PyPI. You can install a different version of ansible-core by . . . "). However, my sense is that for most users version n.n of Ansible "includes" version n.n of ansible-core, because it installs it for them.

If we want to make the user experience more modular, we could make the ansible-core install step separate. But for the average user, my guess is that would be a barrier to use, not an opportunity for greater power or flexibility.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to make the user experience more modular, we could make the ansible-core install step separate.

@acozine you can already have the 'more modular' approach now by installing ansible-core directly and using ansible-galaxy collection install to install the collections you want.

But for the average user, my guess is that would be a barrier to use, not an opportunity for greater power or flexibility.

I agree.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm +1 for merging this as is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, then let's merge this as-is and potentially change this later. Thanks everyone!

"\n")
builder.add_raw_rst(
# noqa: E501
"\n"
Expand Down