Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Missing documentation fix #2516

Merged
merged 1 commit into from Apr 12, 2016
Merged

Missing documentation fix #2516

merged 1 commit into from Apr 12, 2016

Conversation

Petrox
Copy link
Contributor

@Petrox Petrox commented Nov 23, 2015

Extending the documentation to state, that this module does not handle externals well.

 Extending the documentation to state, that this module does not handle externals well.
@gregdek
Copy link
Contributor

gregdek commented Nov 24, 2015

@dsummersl Please review this PR to make sure it adheres to the following guidelines:

http://docs.ansible.com/developing_modules.html#module-checklist

If it passes these guidelines, and if you believe it’s a good PR otherwise, please add a comment with "shipit" in the text, and we will flag it for inclusion.

If it fails these guidelines, please add a comment with "needs_revision" in the text, along with the needed changes, and we will flag it for revision.

(I know this is a pretty simple PR, but would be nice to get validation that this new assertion in the docs is correct and useful.)

@dsummersl
Copy link
Contributor

The change in this PR looks fine WRT the module checklist.

I did notice one additional change that should probably happen though:

  • For password / secret arguments no_log=True should be set: This isn't set for the password documentation section.
  • Requirements should be documented, using the requirements=[] field: The svn requirement is documented under notes, but it looks like that should migrate to requirements.
  • Does module use check_mode? Could it be modified to use it? Document it: It does, its not documented.

Open a separate issues for these?

@gregdek
Copy link
Contributor

gregdek commented Apr 7, 2016

Missed this. @dsummersl I would say open a separate issue for the others; we'll just put this one into shipit per your advice. Thanks @Petrox for the PR.

@dsummersl
Copy link
Contributor

will do.

@Petrox
Copy link
Contributor Author

Petrox commented Apr 10, 2016

@gregdek Although correct docs are important, but I believe implementing the handling of externals would be a better solution, as the current subversion module is just half-working. And the funnyest bug existing is that ansible hits me with warnings for using shell: svn :) 🪲

Maybe in a later release module svn gets a little treatment. 👍

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

Successfully merging this pull request may close these issues.

None yet

4 participants