Conversation
zzantozz
force-pushed
the
wrapper-script
branch
3 times, most recently
from
90cf176 to
6730811
Compare
samwar
left a comment
samwar
left a comment
There was a problem hiding this comment.
A few minor things in the documentation, but overall it LGTM.
| #!/bin/bash -e | ||
|
|
||
| ## This is a script to build docs projects started from the Docs Starter Kit. | ||
| ## This is just a brief shim that relies on the full repository's being installed in a project |
There was a problem hiding this comment.
I'm thinking this is supposed to be plural (or maybe even singular), not possessive.
| ## This is just a brief shim that relies on the full repository's being installed in a project | |
| ## This is just a brief shim that relies on the full repository(ises?) being installed in a project |
There was a problem hiding this comment.
Not sure what you're getting at here... Are you suggesting it should be plural? I don't think so. This is the possessive gerund thing. "Being" is a gerund noun, so "repository's" is a possessive adjective.
There was a problem hiding this comment.
Lol. All I caught was the suggestion. I totally missed the line before it. Still, see gerund noun.
There was a problem hiding this comment.
That just can't be right. What about
| ## This is just a brief shim that relies on the full repository's being installed in a project | |
| ## This is just a brief shim that relies on the full repository installation in a project |
| ## - This wrapper script should be as foolproof as possible because it's intended to be | ||
| ## distributed into all the docs projects, and finding and updating all the copies of it | ||
| ## will be difficult. This is exactly the problem that this independent toolkit is trying to | ||
| ## solve for all the docs projects out there. |
There was a problem hiding this comment.
| ## solve for all the docs projects out there. | |
| ## solve for all of the docs projects out there. |
There was a problem hiding this comment.
Do you find that in the style guide somewhere? I think this is correct either way.
There was a problem hiding this comment.
It probably is just a style difference.
The wrapper script is described in the readme. It's how users and Jenkins will interact with the toolkit. It has to handle every command that this project will support. This shows rudimentary install behavior as well as how the get command works. Other commands are passed through to the internal script. Most of the install logic will also live in the internal script. That script is more complex and left for another PR.
zzantozz
force-pushed
the
wrapper-script
branch
from
6730811 to
88dfc75
Compare
2 participants
The wrapper script is described in the readme. It's how users and
Jenkins will interact with the toolkit. It has to handle every command
that this project will support. This shows rudimentary install
behavior as well as how the get command works. Other commands are
passed through to the internal script. Most of the install logic will
also live in the internal script. That script is more complex and left
for another PR.