-
Notifications
You must be signed in to change notification settings - Fork 2
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
fixup test runner #8
Conversation
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.
good stuff.
proposals/43:upgrade-11/legacy.sh
Outdated
@@ -4,7 +4,6 @@ | |||
source /usr/src/upgrade-test-scripts/env_setup.sh | |||
|
|||
# Enable debugging | |||
set -x | |||
|
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 "Enable debugging" comment stays? on purpose?
not a big deal...
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.
oops, will remove. (CI's not passing anyway)
@@ -29,7 +29,7 @@ ENV UPGRADE_TO=${to} THIS_NAME=${agZeroUpgrade} | |||
# put env functions into shell environment | |||
RUN echo '. /usr/src/upgrade-test-scripts/env_setup.sh' >> ~/.bashrc | |||
|
|||
COPY --chmod=755 ./upgrade-test-scripts /usr/src/upgrade-test-scripts | |||
COPY --link --chmod=755 ./upgrade-test-scripts /usr/src/upgrade-test-scripts |
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.
TIL: COPY --link
fs.writeFileSync('Dockerfile', contents); | ||
} | ||
|
||
if (require.main === module) { |
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 thought we used ES modules and node doesn't support this idiom in that context.
Does .ts not get compiled to ES modules?
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.
Node only uses ESM if package.json
has "type": "module"
. But there's no package.json here so it defaults to CJS
This one got lost in the commit cleanups:
These I'm slipping in as well: