-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Conversation
Happy with the protractor typing fix, but as a note for the future, I don't think that blocking proxy should return a webdriver promise, since it should't be tied to any particular webdriver bindings. |
scripts/generate-docs.sh
Outdated
EXEC_BRANCH=$(git rev-parse --abbrev-ref HEAD) | ||
if [ "$#" -eq 0 ]; then |
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.
Because reading bash is hard, can we add a comment here? Maybe:
# If a commit ref is passed as a command line option, use that. Otherwise, default to the
# latest tagged version.
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.
Done (paraphrased slightly to be technically correct).
Also added usage up top
"minimist", "optimist", "q", | ||
"selenium-webdriver" | ||
] | ||
"outDir": "built/" |
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.
So I assume ts just uses all available typings if there's not an array here?
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.
Essentially. It defaults to everything in node_modules/@types
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.
@juliemr all comments addressed. Changed the line you were concerned about to:
This probably should have been done the whole time, but is unlikely to matter either way at runtime, since blocking proxy wouldn't be returning a webdriver promise regardless.
"minimist", "optimist", "q", | ||
"selenium-webdriver" | ||
] | ||
"outDir": "built/" |
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.
Essentially. It defaults to everything in node_modules/@types
scripts/generate-docs.sh
Outdated
EXEC_BRANCH=$(git rev-parse --abbrev-ref HEAD) | ||
if [ "$#" -eq 0 ]; then |
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.
Done (paraphrased slightly to be technically correct).
Also added usage up top
Also added the ability to run `generate-docs.sh` against a custom branch/tag/commit by doing `./scripts/generate-docs.sh branch_name`. Still defaults to the version from package.json though. Also updated release.md to tell people to check that doc generation/the website is working BEFORE doing a release. Needing a change in the codebase is more likely now that we're compiling down to es5 only this one time, and ideally any changes to the codebase would happen before release. Also including a minor change in the codebase where we wrap a promise from blocking proxy in a webdriver promise. This probably should have been done the whole time, but is unlikely to matter either way at runtime, since blocking proxy wouldn't be returning a webdriver promise regardless. Mostly did I did this to fix typechecking.
Also added the ability to run `generate-docs.sh` against a custom branch/tag/commit by doing `./scripts/generate-docs.sh branch_name`. Still defaults to the version from package.json though. Also updated release.md to tell people to check that doc generation/the website is working BEFORE doing a release. Needing a change in the codebase is more likely now that we're compiling down to es5 only this one time, and ideally any changes to the codebase would happen before release. Also including a minor change in the codebase where we wrap a promise from blocking proxy in a webdriver promise. This probably should have been done the whole time, but is unlikely to matter either way at runtime, since blocking proxy wouldn't be returning a webdriver promise regardless. Mostly did I did this to fix typechecking.
Also added the ability to run
generate-docs.sh
against a custombranch/tag/commit by doing
./scripts/generate-docs.sh branch_name
. Stilldefaults to the version from package.json though.
Also updated release.md to tell people to check that doc generation/the website
is working BEFORE doing a release. Needing a change in the codebase is more
likely now that we're compiling down to es5 only this one time, and ideally
any changes to the codebase would happen before release.
Also including a minor change in the codebase where we wrap a promise from
blocking proxy in a webdriver promise. This probably should have been done the
whole time, but is unlikely to matter either way at runtime, since blocking
proxy wouldn't be returning a webdriver promise regardless. Mostly did I did
this to fix typechecking.