-
Notifications
You must be signed in to change notification settings - Fork 1.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
Correct spelling errors in the docs #256
Conversation
The Beijing, Sevilla, Albany and Austin MUGs no longer exist on Meetup, removing.
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.
Looks good! Just a couple of suggestions for "iff" and "acknowledgements".
@@ -20,7 +20,7 @@ Query parameters: | |||
|
|||
|
|||
### AUTHENTICATION ### | |||
This endpoint requires authentication iff HTTP authentication is | |||
This endpoint requires authentication if HTTP authentication is |
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.
We should leave as 'iff' since this is a short form of "if and only if". Ditto on the other cases.
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.
Since it is documentation, would it be better not to use acronym ? So that translation of the documentation maybe access by machine or the reader easier.
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.
Sure, however this file is actually generated from source, so the change would need to be made there:
https://github.com/apache/mesos/blob/1.4.0/src/files/files.cpp#L352-L371
https://github.com/apache/mesos/blob/1.4.0/3rdparty/libprocess/include/process/help.hpp#L82-L90
I think @bbannier was looking into whether we could remove generated markdown since they're no longer needed. We now generate them at the time of publishing the website.
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 created https://issues.apache.org/jira/browse/MESOS-8442 to track removal of these files; the fix is in principle trivial, we just need to make sure to not break existing tooling.
Regarding this change, I was educated by a native speaker that the spelling here is actually valid and intended when I tried to perform a similar change, https://reviews.apache.org/r/46934/.
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.
Noted, I wasn't familiar with this short form. I'll remove these edits.
@@ -91,13 +91,13 @@ virtual void offerRescinded(SchedulerDriver* driver, const OfferID& offerId); | |||
* Invoked when the status of a task has changed (e.g., a slave is | |||
* lost and so the task is lost, a task finishes and an executor | |||
* sends a status update saying so, etc). If implicit | |||
* acknowledgements are being used, then returning from this | |||
* acknowledgments are being used, then returning from this |
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 inclined to leave in the extra "e" since I believe both spellings are acceptable FWICT and we have more with the extra "e":
➜ mesos git:(master) ✗ grep -Ri acknowledgements src | wc -l
149
➜ mesos git:(master) ✗ grep -Ri acknowledgments src | wc -l
6
Ditto for the other cases.
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.
Ok, I'll back out these adjustments.
Being more familiar with Gerrit than GitHub with forks, I may have made a bit of a mess here, LMK if you want me to untangle this further or create a new PR, but I think it's generally where we need to be. |
Hm.. I don't know if I'm looking at this right, but when I click 'Files Changed' I still see the "iff" and "acknowledgements" adjustments, so I wonder if your update took effect or if I'm not looking at the diff correctly? |
It seems the diff is unhelpfully showing the difference between the two commits, rather than from the source. Pulling it down to do a local squash and diff should show things properly. |
Hm.. I'm unable to pull the patch down:
|
What a mess. I'll fiddle with my patch and see about submitting it over on the Review Board side. |
Sorry about that, and thank you for your patience! |
No description provided.