-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
docker_swarm_service: rename return variable to swarm_service #53229
docker_swarm_service: rename return variable to swarm_service #53229
Conversation
ff8a330
to
28d0fda
Compare
@hannseman @jwitko please say something if you don't like this change :) (the new name was suggested by @dariko, see here and below). Or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@abadger I assume you're still the release manager for Ansible 2.7.x. Can you take a look at this PR and say if it is ok from your point of view? Otherwise it doesn't make sense to merge it in this form (and backport it to stable-2.7). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
After talking to @abadger, I've added a remark that the old name will stay for Ansible 2.7.x. I will add that in the backport PR once this is merged. |
ready_for_review |
This change does not require the cycle of four releases of maintaining old name? |
This is kind of special, since the return value never worked as documented, we can consider this feature as "not really used". Keeping it working in 2.7.x is a courtesy to people who figured out that the result is called differently (but apparently didn't report this discrepancy). |
shipit |
@hannseman @jwitko @WojciechowskiPiotr thanks for your feedback! |
SUMMARY
As discussed in #51939, renaming the return variable to
swarm_service
.ISSUE TYPE
COMPONENT NAME
docker_swarm_service