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

fix k8s_exec, returning rc attribute to follow ansible's common return values. #230

Merged
merged 1 commit into from
Sep 13, 2021

Conversation

itaru2622
Copy link
Contributor

SUMMARY

fix #229.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

k8s_exec

ADDITIONAL INFORMATION

@abikouo
Copy link
Contributor

abikouo commented Sep 9, 2021

@itaru2622 thanks for taking the time to submit a PR for this issue.
I would rather suggest to add the rc as output to be compliant with ansible documentation and aligned with others modules like helm_plugin. To avoid breaking changes you should also keep the ```return_code`` value.
@Akasurde @gravesm WDYT ?

@gravesm
Copy link
Member

gravesm commented Sep 9, 2021

@itaru2622 thanks for the PR. As @abikouo said, we need to keep the existing return_code value for backwards compatibility. Just add your new rc value to the return as well. For the RETURN documentation block you can just copy what is there for return_code but could you also add a version_added: 2.3.0? We'll also need a changelog fragment.

@abikouo abikouo added the has_issue This PR has a related issue it could close. label Sep 9, 2021
@itaru2622
Copy link
Contributor Author

itaru2622 commented Sep 9, 2021

@abikouo @gravesm thank you for your comment. I understand about backward compatibility.

I will keep return_code, but mark it as "deprecate" in document, because it will be dupulicated with 'rc'. Is it acceptable for you?

@itaru2622 itaru2622 changed the title fix k8s_exec's return value, return_code=>rc, to follow ansible's common return values fix k8s_exec, returning rc attribute to follow ansible's common return values. Sep 10, 2021
@itaru2622
Copy link
Contributor Author

itaru2622 commented Sep 10, 2021

@abikouo @gravesm I applied your comments. could you review again, please.

  • keep return_code and add rc
  • add changelog fragments
  • added_version: 2.3.0 for rc in RETURN document.

I also marked 'deprecate' on return_code.

@itaru2622
Copy link
Contributor Author

@abikouo cloud you merge this PR into main branch?
or I need something after approved ?

@abikouo
Copy link
Contributor

abikouo commented Sep 13, 2021

@abikouo cloud you merge this PR into main branch?
or I need something after approved ?

@itaru2622 everything is ok now, thanks for your contribution, it is really appreciate

@abikouo abikouo added mergeit and removed has_issue This PR has a related issue it could close. labels Sep 13, 2021
Copy link

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

k8s_exec's return value has no rc but 'return_code'
3 participants