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.

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

Suggest adt installation #662

Merged
merged 7 commits into from
Apr 22, 2024
Merged

Suggest adt installation #662

merged 7 commits into from
Apr 22, 2024

Conversation

audgirka
Copy link
Contributor

Suggest ADT installation

Jira-https://issues.redhat.com/browse/AAP-19818

@audgirka audgirka requested a review from a team as a code owner March 20, 2024 10:13
@github-actions github-actions bot added docs Changes to documentation needs_triage New item that needs to be triaged labels Mar 20, 2024
@audgirka audgirka force-pushed the fix/adt_install branch 5 times, most recently from c8fcc72 to b41445c Compare March 20, 2024 11:19
@ssbarnea
Copy link
Member

@audgirka This is not rendering correct, see https://ansible--662.org.readthedocs.build/projects/builder/en/662/installation/ correct the RST syntax.

@audgirka audgirka force-pushed the fix/adt_install branch 3 times, most recently from af6171d to 56763f9 Compare March 26, 2024 15:08
docs/installation.rst Outdated Show resolved Hide resolved
docs/installation.rst Outdated Show resolved Hide resolved
@audgirka audgirka force-pushed the fix/adt_install branch 4 times, most recently from 4738fe3 to 9b889de Compare March 26, 2024 15:38
docs/installation.rst Outdated Show resolved Hide resolved
Copy link
Member

@sivel sivel left a comment

Choose a reason for hiding this comment

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

Hi, thank you for your interest in ansible-builder.

We've looked over this proposed change, however we would like to see some of the wording changed.

Instead of recommending this as the preferred way to install ansible-builder, we believe it should be presented as just an additional option. The reasoning for the decision making as to why a user would want to use this installation option, that includes many more tools than just builder should be explained, and also indicated what audience may be best served by doing so. However, this should probably be a link to the ansible-dev-tools repo or documentation, where that information can be explained in more detail.

@audgirka
Copy link
Contributor Author

audgirka commented Mar 26, 2024

Hi, thank you for your interest in ansible-builder.

We've looked over this proposed change, however we would like to see some of the wording changed.

Instead of recommending this as the preferred way to install ansible-builder, we believe it should be presented as just an additional option. The reasoning for the decision making as to why a user would want to use this installation option, that includes many more tools than just builder should be explained, and also indicated what audience may be best served by doing so. However, this should probably be a link to the ansible-dev-tools repo or documentation, where that information can be explained in more detail.

Will look at the wording changes for the "recommended" approach.
We've tried to keep the same message across the other projects included as part of the adt package.

Line 20 onwards explains briefly the use case for ansible-dev-tools
https://github.com/ansible/ansible-builder/pull/662/files#diff-d9b149498982c0663c3b7170398773361ed5678f1a627e9c2fd8d2c955c563dbR20

We do have a link in place for adt docs which can help explain user the purpose of using this package and tools included as part of it.

@ssbarnea @cidrblock ^^

@audgirka audgirka requested a review from ssbarnea March 26, 2024 17:19
@cidrblock
Copy link

Let's update the wording per @sivel request so we can get this in. If it needs to be revised further in the futre we can do that at a later date.

Copy link
Contributor

@oraNod oraNod left a comment

Choose a reason for hiding this comment

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

Left some suggestions. Feel free to take them, leave them, or modify them however you think. Hope this helps!

docs/installation.rst Outdated Show resolved Hide resolved
docs/installation.rst Outdated Show resolved Hide resolved
@ssbarnea
Copy link
Member

@sivel Please take another look, that should be the text we agreed on ("alternative"), renders ok, it has the note and no extra heading.

You might want to retrigger the failed job, which fails for unrelated reason. I cannot do it myself.

@audgirka audgirka closed this Mar 28, 2024
@audgirka audgirka reopened this Mar 28, 2024
@audgirka audgirka requested review from bcoca, sivel and oraNod April 8, 2024 13:20
docs/installation.rst Outdated Show resolved Hide resolved
docs/installation.rst Outdated Show resolved Hide resolved
docs/installation.rst Outdated Show resolved Hide resolved
docs/installation.rst Outdated Show resolved Hide resolved
@sivel sivel merged commit f940906 into ansible:devel Apr 22, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Changes to documentation needs_triage New item that needs to be triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants