-
Notifications
You must be signed in to change notification settings - Fork 122
Combine environment setting steps into one step with shell script #1010
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
Conversation
❌ Integration test FAILEDRequested by @sunmou99 on commit 5ccf419
Add flaky tests to go/fpl-cpp-flake-tracker |
- uses: actions/checkout@v2 | ||
with: | ||
ref: ${{needs.check_and_prepare.outputs.github_ref}} | ||
submodules: true | ||
- name: Store git credentials for all git commands |
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.
Is this step still happening? It's important for the GitHub runners to store the token for all git commands.
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.
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.
Why it's important, e.g. what will happen if we don't have it? And does step the order matters?
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.
This helps us with the issue where GitHub would throttle anonymous Git requests (requests without a token). It's important when downloading external projects, fetching cocoapods, etc. (#727)
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.
LGTM - this doesn't affect packaging, right?
Yeah, this only affects the testing part. |
Description
Removed some environment setting steps, now we are using the following script to set Integration Test CI environments:
python scripts/gha/install_prereqs.sh - ${platform}
Other experiments :
bash.exe
as the shell on Windows runner.git
command line can be different. (source)raise UnidiffParseError('Unexpected trailing newline character')
if the PR change end with a "file name change without file content change". (example)Testing
See the comment below
Type of Change
Place an
x
the applicable box:Notes
Release Notes
section ofrelease_build_files/readme.md
.