-
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
Clarify docs re mode's octal representation #44410
Conversation
I changed the language about how to use mode, calling attention to the fact that one is *adding* a zero to mark it as octal to Python, rather than simply including the zero that one might have put there when using the chmod command on a Unix command line. This makes it more obvious that using "01777" is not a typo, because the leading zero is not meant to reflect the way that number might have been given on a command line. As pointed out in PEP 3127, "The default octal representation of integers is silently confusing to people unfamiliar with C-like languages." That PEP is standards track with final status. See also: issues #5409 #9196 #11385 #13115 #18952 #23491 #23521
@lousyd it is strongly advised to fill in the PR template with relevant information when submitting PRs. |
lib/ansible/modules/files/file.py
Outdated
- file: | ||
path: /etc/foo.conf | ||
owner: foo | ||
group: foo | ||
mode: 0644 | ||
# when specifying mode using octal numbers, add a leading 0 | ||
- file: |
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.
comments should go above the thing they're commenting. you can put it above the mode line, though
@acozine the change here doesn't make it any clearer to me but I'm not against it either. if you like the new wording feel free to merge. if you can think of something better, that would be fine with me as well. |
lib/ansible/modules/files/file.py
Outdated
- file: | ||
path: /etc/foo.conf | ||
owner: foo | ||
group: foo | ||
mode: 0644 | ||
# when specifying mode using octal numbers, add a leading 0 |
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.
I would put this back where it was, above the mode
line, or on the mode
line off to the side. Definitely not below.
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.
Just to clarify, I meant for that comment to be above the thing it was commenting. In other words, that comment was meant to refer to the next play. But since this is the second mention of it, maybe it's confusing.
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.
I re-modified lib/ansible/modules/files/file.py to move that comment up above the examples.
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.
I'm okay with this change. While you're clarifying, @lousyd, I'd like to see the next section changed from:
"or quote it (like C('644') or C('0644')"
to be:
"or quote it (like C('644') or C('1777')"
so it's clear that you can either do 01777
or '1777'
.
lib/ansible/modules/files/file.py
Outdated
@@ -79,7 +79,7 @@ | |||
''' | |||
|
|||
EXAMPLES = ''' | |||
# change file ownership, group and mode. When specifying mode using octal numbers, first digit should always be 0. | |||
# change file ownership, group and mode. When specifying mode using octal numbers, add a leading 0 |
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.
The idea was to put 0
reference just above mode
argument:
# here:
mode: 0644
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.
That makes a lot more sense to me than what I was thinking you wanted. Thank you. I've made that change.
thanks @lousyd for seeing this through! |
I changed the language about how to use mode, calling attention to the
fact that one is adding a zero to mark it as octal to Python, rather
than simply including the zero that one might have put there when using
the chmod command on a Unix command line. This makes it more obvious
that using "01777" is not a typo, because the leading zero is not meant
to reflect the way that number might have been given on a command line.
As pointed out in PEP 3127, "The default octal representation of
integers is silently confusing to people unfamiliar with C-like
languages." That PEP is standards track with final status.
See also: issues #5409 #9196 #11385 #13115 #18952 #23491 #23521