Skip to content
This repository was archived by the owner on Apr 7, 2026. It is now read-only.

chore: mark native image checks as required#1930

Merged
rajatbhatta merged 1 commit intomainfrom
require-graal-check
Dec 13, 2022
Merged

chore: mark native image checks as required#1930
rajatbhatta merged 1 commit intomainfrom
require-graal-check

Conversation

@mpeddada1
Copy link
Copy Markdown
Contributor

@mpeddada1 mpeddada1 commented Jun 29, 2022

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> ☕️

If you write sample code, please follow the samples format.

@mpeddada1 mpeddada1 requested review from a team June 29, 2022 14:36
@product-auto-label product-auto-label Bot added size: xs Pull request size is extra small. api: spanner Issues related to the googleapis/java-spanner API. labels Jun 29, 2022
@ansh0l
Copy link
Copy Markdown
Contributor

ansh0l commented Jul 4, 2022

Hey @mpeddada1 , would you have any stats on how this new presubmit is doing? As in, how often was it failing when any of the merges got done previously? If not, I would want to monitor for an additional month or so before making it mandatory.

@ansh0l ansh0l added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 4, 2022
@mpeddada1
Copy link
Copy Markdown
Contributor Author

mpeddada1 commented Jul 6, 2022

Hey @ansh0l, just looked into the recently merged PRs in this repository. I haven't observed failures specific to native image compilation so far but I have noticed the ITInstanceAdminTest#updateInstance and ITInstanceAdminTest#updateInstanceViaEntity tests in these presubmits being pretty flaky. I'm wondering if this could be related to #1626?

@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 8, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 8, 2022
@ansh0l
Copy link
Copy Markdown
Contributor

ansh0l commented Jul 12, 2022

cc @rajatbhatta to take a look at the related issue

@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 12, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 12, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@yoshi-kokoro yoshi-kokoro removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jul 13, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 13, 2022
@mpeddada1
Copy link
Copy Markdown
Contributor Author

mpeddada1 commented Jul 18, 2022

Thanks @ansh0l! are we considering #1626 as a prerequisite to marking these checks as required?

This was referenced Jul 19, 2022
Merged
Merged
@mpeddada1
Copy link
Copy Markdown
Contributor Author

mpeddada1 commented Aug 24, 2022

Hi @ansh0l, checking back on this PR. Now that #1930 is fixed, are we okay to remove the do-merge-label?

@ansh0l
Copy link
Copy Markdown
Contributor

ansh0l commented Aug 29, 2022

Thanks @mpeddada1 ! We can monitor for another couple of weeks, and then merge this.

@mpeddada1
Copy link
Copy Markdown
Contributor Author

mpeddada1 commented Dec 13, 2022 •
edited
Loading

Hey @ansh0l @rajatbhatta looks like we haven't had a chance to merge this PR in yet. Could we get your review on this? It'll help us gate releases (without manual verification) on the compatibility with native image compilation.

@rajatbhatta rajatbhatta removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 13, 2022
@rajatbhatta rajatbhatta merged commit 3a8ceb4 into main Dec 13, 2022
@rajatbhatta rajatbhatta deleted the require-graal-check branch December 13, 2022 16:06
@rajatbhatta
Copy link
Copy Markdown
Contributor

rajatbhatta commented Dec 13, 2022

Hi @mpeddada1: Merged this PR. Thank you!

@mpeddada1
Copy link
Copy Markdown
Contributor Author

mpeddada1 commented Dec 20, 2022

Thank you @rajatbhatta!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ansh0l ansh0l Awaiting requested review from ansh0l

1 more reviewer

@rajatbhatta rajatbhatta rajatbhatta approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: spanner Issues related to the googleapis/java-spanner API. size: xs Pull request size is extra small.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mpeddada1 @ansh0l @rajatbhatta @yoshi-kokoro