Skip to content

Add ci.yaml for github workflows #1

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

Closed
wants to merge 3 commits into from
Closed

Conversation

jylinv0
Copy link
Collaborator

@jylinv0 jylinv0 commented Jan 26, 2024

Add instrumentation test to github workflow

@jylinv0 jylinv0 force-pushed the workflow branch 3 times, most recently from d0f860f to 86e38fd Compare February 6, 2024 20:46
@jylinv0 jylinv0 marked this pull request as draft February 6, 2024 21:12
@@ -34,6 +34,9 @@ SED_SCRIPT=${TEST_TMPDIR}/runner_config.sed
# enums.
echo 's#\\*\"@!([^!]+)!@\\*\"#\1#g' > ${SED_SCRIPT}

unset rlocation
unset is_absolute
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this doing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're functions that created by bazel during test. The generation of sed script doesn't take care of function, so the script will be formatted incorrectly. The proper fix would be explicitly name which env vars to print in printenv or somehow handle functions properly during the generation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment explaining this, or adjust the grep expression used to remove lines from the printenv output and add the comment there. It'd be very difficult to know ahead of time exactly which environment variables we want to expand.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments.

@copybara-service copybara-service bot closed this in 29fdfe8 Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants