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
notify: store resulting commit in PullRequestState #641
Conversation
|
Webrevs
|
Looks good, but may need to add an extra configuration field.
var prIssues = new PullRequestState(pr, issues); | ||
var current = storage.current(); | ||
if (current.contains(prIssues)) { | ||
var commit = resultingCommitHashFor(pr, pr.repository().forge().currentUser()); |
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 notifier bot may not be running with the same credentials as the commit bot, so this is possibly the wrong user to look for. The other bots (mlbridge, pr) takes a username from the configuration to use for matching comment poster ids.
@edvbld This change now passes all automated pre-integration checks. When the change also fulfills all project specific requirements, type
Since the source branch of this PR was last updated there have been 2 commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid automatic rebasing, please merge
|
/integrate |
Hi all,
please review this patch that the resulting (integrated) commit from a PR in the
PullRequestState
. This data will be used in future patches for more easier handling of notifications. Note that all data in theStorage
might not have the"commit"
field. If that is the case, then the code will lazily populate the"commit"
field.Testing:
make test
on Linux x64Thanks,
Erik
Progress
Reviewers
Download
$ git fetch https://git.openjdk.java.net/skara pull/641/head:pull/641
$ git checkout pull/641