Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: create pr after library generation #10503
feat: create pr after library generation #10503
Changes from 38 commits
a18841c
949c779
fbdf767
5c3b52f
f1fb22f
87d9c3a
03af469
86f3b9b
e859513
03193a3
f5e36ac
84b5543
668f6af
087982f
8f2f6cf
2702a5c
adbeb2a
9f9a7b4
b65be6f
eabbc07
ebffdbd
8f955d7
41cc413
d0d0cdb
8acbc80
0862a24
746676f
7f7c38d
8b12617
8d698ab
df44e18
f9d7e7e
4e78e89
6acc593
d58bdc2
1752882
0211cfe
d336503
23190c4
6815a40
400025a
fd27bab
8698923
554938f
c964025
a1dc0ce
b103130
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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'll remove this trigger after pr is approved.
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.
Ideally we should not use
latest
, but rather a versioned docker image, otherwise it is not hermetic. But since this is the first time we use the scripts and there might be bug fixes in this cycle, let's keep it for now. Once the scripts are stable, we should read the gapic-generator-java version from the build config file and use that version for the docker image.For now, can we extract
latest
as a parameter and share it between this step and the step below?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.
Just FYI, we have a release job that releases
gcr.io/cloud-devrel-public-resources/java-library-generation:${GGJ_VERSION}
, so if we are to remove it, we can use that tag (and configure renovate bot)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 think we should group the docker image tag with generator update so that renovate bot can update them together.
Sounds good.
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.
My instinct says "chore:" does not goes to Release Please pull request and the pull request description is not read by Release Please. Let's see.
(If this is really the case, we can fix it by "BEGIN_COMMIT_OVERRIDE".)
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 feel the same way. Can we make sure we understand the behavior before merging? e.g. If it is a
chore
, is release please going to include the NESTE_COMMIT in the description? If it is afeat
, is the title going to be included? The behavior of release-please will affect how we generate the PR descriptions.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.
According to @chingor13, release-please won't skip a commit if it's a chore, it just ignores chore commit messages.
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.
"chore" appears twice.
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.
Updated the commit message.
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 do we still need
owlbot:run
here?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 check is not removed yet.
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 think we should remove the label here and disable the check for this branch, otherwise owlbot postprocessing would still be run against this PR right?
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.
+1 to removing the required check for owlbot postprocessor and the label
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.
Done.
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.
Which script do you use to populate this yaml?
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 manually added these libraries, the parameters are copied from new library generation PR.