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

ansible-doc: restore role attributes #82678

Merged
merged 4 commits into from
Mar 21, 2024
Merged

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Fixes #82639.

ISSUE TYPE
  • Bugfix Pull Request

@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. has_issue needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 12, 2024
@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Feb 13, 2024
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Feb 13, 2024
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Feb 27, 2024
@sivel
Copy link
Member

sivel commented Mar 12, 2024

@felixfontein we've had a discussion around re-adding this unexpected feature. Since we didn't have any documentation explaining that unintended features could be removed without deprecation, we would be willing to potentially re-accept this.

However, the stipulation being that, it would be merged with an immediate deprecation for future removal.

With the above being said, do you feel it would be useful to re-add this feature to start immediately preparing to stop using it, or should we just leave it removed?

@felixfontein
Copy link
Contributor Author

@sivel while I still disagree that it should be removed/deprecated at all, having a proper deprecation period is better than just removing it.

@s-hertel
Copy link
Contributor

I added a deprecation warning so this can be merged, hopefully this looks okay:

[DEPRECATION WARNING]: The role testns.testcol.testrole's argument spec alternate contains the key "attributes", which will not be displayed by ansible-doc in the future. This was unintentionally allowed when plugin attributes were
added, but the feature does not map well to role argument specs. This feature will be removed from ansible-core in version 2.20. Deprecation warnings can be disabled by setting deprecation_warnings=False in ansible.cfg.

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 14, 2024
@felixfontein
Copy link
Contributor Author

This also does need a deprecation docs fragment now. @s-hertel do you want to add that as well, or should I?

@s-hertel
Copy link
Contributor

Since there was no changelog/documentation for the accidental feature, I didn't think it needed one, but if you want to add one feel free, I'm out until Monday.

@bcoca
Copy link
Member

bcoca commented Mar 15, 2024

I would add a changelog since it includes a deprecation now

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 15, 2024
@felixfontein

This comment was marked as resolved.

@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 18, 2024
@bcoca bcoca merged commit 11d69e0 into ansible:devel Mar 21, 2024
62 checks passed
@felixfontein felixfontein deleted the role-attributes branch March 21, 2024 21:22
@felixfontein
Copy link
Contributor Author

@s-hertel @bcoca thanks for reviewing and merging!

@ansible ansible locked and limited conversation to collaborators Apr 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug. has_issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: ansible-doc no longer shows attributes for roles
6 participants