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

files: fix docs about type of the mode parameter #61232

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

@sourcejedi
Copy link

@sourcejedi sourcejedi commented Aug 23, 2019

SUMMARY

mode is an octal or symbolic mode, for the unix file permission. It is
not a path, as the docs for the copy module suggest.

Document the various mode parameters as being of type as "raw".

The directory_mode parameter of the copy module was already of type
"raw". Perhaps this looks a bit weird, but the documentation text for
mode is very explicit about how it allows both integer and string values.

ansible-test sanity --test validate-modules missed some inconcistencies
between the docs and the code due to how the common file arguments work.
(I noticed it yelled at me if I introduce a discrepancy elsewhere).
I think this change removes inconsistencies.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

copy

ADDITIONAL INFORMATION

n/a

@ansibot
Copy link
Contributor

@ansibot ansibot commented Aug 23, 2019

Loading

@ansibot
Copy link
Contributor

@ansibot ansibot commented Aug 23, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/files/copy.py:0:0: E305 DOCUMENTATION.options.directory_mode.type: not a valid value for dictionary value @ data['options']['directory_mode']['type']. Got 'string'
lib/ansible/modules/files/copy.py:0:0: E305 DOCUMENTATION.options.mode.type: not a valid value for dictionary value @ data['options']['mode']['type']. Got 'string'
lib/ansible/modules/files/copy.py:0:0: E325 Argument 'directory_mode' in argument_spec defines type as 'raw' but documentation defines type as 'string'

click here for bot help

Loading

@sourcejedi sourcejedi changed the title copy: fix docs about type of the mode parameter files: fix docs about type of the mode parameter Aug 23, 2019
@sourcejedi
Copy link
Author

@sourcejedi sourcejedi commented Aug 23, 2019

Thanks @ansibot! I've re-written it, including the PR description.

Loading

sourcejedi added 2 commits Aug 24, 2019
`mode` is an octal or symbolic mode, for the unix file permission.  It is
not a path, as the docs for the copy module suggest.

Document the various mode parameters as being of type as "raw".

The `directory_mode` parameter of the copy module was already of type
"raw".  Perhaps this looks a bit weird, but the documentation text for
`mode` is very explicit about how it allows both integer and string values.

`ansible-test sanity --test validate-modules` missed some inconcistencies
between the docs and the code due to how the common file arguments work.
(I noticed it yelled at me if I introduce a discrepancy elsewhere).
I think this change removes inconsistencies.
GitHub code search suggests no-one refers to this.  Including us
(or our docs).

We know what happens to dead code.  Let's send it to the bitbucket in the
sky.
@sourcejedi
Copy link
Author

@sourcejedi sourcejedi commented Aug 24, 2019

I pushed the wrong version :-(. Third time lucky?

Loading

@sourcejedi
Copy link
Author

@sourcejedi sourcejedi commented Aug 24, 2019

Ok, maybe the remaining test failures are not my fault? I couldn't find any relevant errors. Test 75 looks like a failure to contact the repo, and 111 just took a little too long. I don't understand where the error is in 85.

Loading

@acozine
Copy link
Contributor

@acozine acozine commented Aug 28, 2019

@sourcejedi thank you for your interest in making the Ansible documentation the best it can be!

I'm not sure this is the right fix, though - why did you remove the shared argspec?

Loading

@sourcejedi
Copy link
Author

@sourcejedi sourcejedi commented Aug 28, 2019

Hi again @acozine. It's because I don't think the argspec is shared by anything :-).

If you're worried about the few test failures, I can split it into a separate PR so we can confirm that's not the problem. Or you can tell me this opportunistic refactoring needs to be considered as a separate PR :-). The other commit does not depend on it.

module_utils: remove unused function get_file_args_spec()

GitHub code search suggests no-one refers to this. Including us
(or our docs).

We know what happens to dead code. Let's send it to the bitbucket in the
sky.

Loading

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

Successfully merging this pull request may close these issues.

None yet

3 participants