Skip to content
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

Invoke bazel build during sync using --target_pattern_fi… #6595

Merged

Conversation

sfc-gh-mgalindo
Copy link
Contributor

Use --target_pattern_file flag during sync instead of passing list of targets to avoid large command line errors in the same spirit as #6537.

…le flag

Description
JIRA: SNOW-1482917

Use --target_pattern_file flag during sync instead of passing list of targets
to avoid large command line errors in the same spirit as bazelbuild#6537.

Testing
@sfc-gh-mgalindo sfc-gh-mgalindo marked this pull request as draft July 25, 2024 00:57
@github-actions github-actions bot added product: CLion CLion plugin product: IntelliJ IntelliJ plugin product: GoLand GoLand plugin awaiting-review Awaiting review from Bazel team on PRs labels Jul 25, 2024
@tpasternak
Copy link
Collaborator

@sfc-gh-mgalindo it's still in draft mode, would you like us to review it?

@sfc-gh-mgalindo
Copy link
Contributor Author

@sfc-gh-mgalindo it's still in draft mode, would you like us to review it?

@tpasternak yes, please review. Marked it as ready for review now

@tpasternak tpasternak requested review from jastice and removed request for jastice July 26, 2024 21:42
private static @NotNull Path prepareTargetPatternFile(List<? extends TargetExpression> targets) throws BuildException {
Path targetPatternFile = null;
try {
targetPatternFile = Files.createTempFile("targets-", "");
Copy link
Collaborator

Choose a reason for hiding this comment

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

@tpasternak
Copy link
Collaborator

@sfc-gh-mgalindo Thank you for the contribution! I agree with Mai, could you please put the temporary in the same directory as the previous one? Apart from that LGTM

@sfc-gh-mgalindo
Copy link
Contributor Author

@sfc-gh-mgalindo Thank you for the contribution! I agree with Mai, could you please put the temporary in the same directory as the previous one? Apart from that LGTM

Done.

@tpasternak
Copy link
Collaborator

Thanks, please only fix the compilation errors and we'll merge it

@sfc-gh-mgalindo
Copy link
Contributor Author

Thanks, please only fix the compilation errors and we'll merge it

Thanks!

@tpasternak
Copy link
Collaborator

tpasternak commented Aug 19, 2024

@satyanandak may I ask you to rerun the CI (one of the test suites did not start, so I can't merge the PR)

@tpasternak tpasternak merged commit 09a186b into bazelbuild:master Aug 19, 2024
6 checks passed
@github-actions github-actions bot removed the awaiting-review Awaiting review from Bazel team on PRs label Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product: CLion CLion plugin product: GoLand GoLand plugin product: IntelliJ IntelliJ plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants