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

Use tag annotation if present when sending notifications #243

Closed
wants to merge 4 commits into from

Conversation

@rwestberg
Copy link
Member

rwestberg commented Nov 7, 2019

Hi all,

Please review this change that updates the notifier to include the tag annotation if it is present when creating a new tag notification email. Also ensure that notifications are sent even if a tag does not conform to the OpenJDK tag parser format.

Best regards,
Robin

Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed

Approvers

…notation info if present
@bridgekeeper
Copy link

bridgekeeper bot commented Nov 7, 2019

👋 Welcome back rwestberg! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request (refresh this page to view it).

@openjdk openjdk bot added rfr bots labels Nov 7, 2019
@mlbridge
Copy link

mlbridge bot commented Nov 7, 2019

@edvbld
edvbld approved these changes Nov 8, 2019
Copy link
Member

edvbld left a comment

Looks good!

@openjdk openjdk bot removed the rfr label Nov 8, 2019
@openjdk
Copy link

openjdk bot commented Nov 8, 2019

@rwestberg This change can now be integrated. The commit message will be:

Use tag annotation if present when sending notifications

Reviewed-by: ehelin
  • If you would like to add a summary, use the /summary command.
  • To list additional contributors, use the /contributor command.

Since the source branch of this PR was last updated there has been 1 commit pushed to the master branch:

  • 069620b: Add buildnum extraction support for OpenJFX tags

Since there are no conflicts, your changes will automatically be rebased on top of the above commits when integrating. If you prefer to do this manually, please merge master into your branch first.

  • To integrate this PR with the above commit message, type /integrate in a new comment.
@openjdk openjdk bot added the ready label Nov 8, 2019
@edvbld
edvbld approved these changes Nov 8, 2019
Copy link
Member

edvbld left a comment

Still good, just a small suggestion inline based on latest changes 😺

var newNonJdkTags = newTags.stream()
.filter(tag -> OpenJDKTag.create(tag).isEmpty());
.filter(tag -> OpenJDKTag.create(tag).isEmpty())
.collect(Collectors.toList());

This comment has been minimized.

@edvbld

edvbld Nov 8, 2019 Member

Please move this below the for loop so it is next to the for loop of newNonJdkTags

This comment has been minimized.

@rwestberg

rwestberg Nov 8, 2019 Author Member

Sure!

@kevinrushforth
Copy link
Member

kevinrushforth commented Nov 8, 2019

I look forwarded to testing this once you've updated the bot with this change.

@rwestberg
Copy link
Member Author

rwestberg commented Nov 8, 2019

/integrate

@openjdk openjdk bot closed this Nov 8, 2019
@openjdk openjdk bot added integrated and removed ready labels Nov 8, 2019
@openjdk
Copy link

openjdk bot commented Nov 8, 2019

@rwestberg The following commits have been pushed to master since your change was applied:

  • 069620b: Add buildnum extraction support for OpenJFX tags

Your commit was automatically rebased without conflicts.

Pushed as commit 2cb3a2e.

@mlbridge
Copy link

mlbridge bot commented Nov 8, 2019

Mailing list message from Robin Westberg on skara-dev:

Changeset: 2cb3a2e
Author: Robin Westberg
Date: 2019-11-08 08:39:57 +0000
URL: https://git.openjdk.java.net/skara/commit/2cb3a2e4

Use tag annotation if present when sending notifications

Reviewed-by: ehelin

! bots/notify/src/main/java/org/openjdk/skara/bots/notify/JNotifyBot.java
! bots/notify/src/main/java/org/openjdk/skara/bots/notify/JsonUpdater.java
! bots/notify/src/main/java/org/openjdk/skara/bots/notify/MailingListUpdater.java
! bots/notify/src/main/java/org/openjdk/skara/bots/notify/UpdateConsumer.java
! bots/notify/src/test/java/org/openjdk/skara/bots/notify/UpdaterTests.java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.