Bi 168 add a way to get to jira tickets from GitHub #8

tungledao · 2017-08-09T09:41:21Z

No description provided.

manhtran1609 · 2017-08-18T04:15:30Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+			LOG.debug("Pull request title " + branch_name  + " does not contain any ticket of Cozmo project");
+		}
+		while (cozmo_matcher.find()){
+			String coz_id = cozmo_matcher.group().replaceAll("coz", jira_link + "coz-");


why is coz- here? I think it is cozmo-

manhtran1609 · 2017-08-18T04:17:35Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+
+		Pattern cozmo_pattern = Pattern.compile(cozmo);
+		Matcher cozmo_matcher = cozmo_pattern.matcher(name);
+		if (!od_matcher.matches()){


need a space between ) and {

manhtran1609 · 2017-08-18T04:17:56Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+		if (!od_matcher.matches()){
+			LOG.debug("Pull request title " + branch_name  + " does not contain any ticket of Cozmo project");
+		}
+		while (cozmo_matcher.find()){


need a space between ) and {

manhtran1609 · 2017-08-18T04:19:09Z

...r-server/src/main/java/jetbrains/buildServer/commitPublisher/github/ChangeStatusUpdater.java

+            final String vcsBranch = version.getVcsBranch();
+            String title = "";
+            try {
+                title = api.getPullRequestTitle(repositoryOwner, repositoryName, vcsBranch);


should follow 2 space instead of 4

manhtran1609 · 2017-08-18T04:19:45Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+public class GetJiraTickets {
+	private static final Logger LOG = Logger.getInstance(GetJiraTickets.class.getName());


should follow 2 space instead of 4

manhtran1609 · 2017-08-18T04:20:27Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+
+		Pattern bi_pattern = Pattern.compile(bi);
+		Matcher bi_matcher = bi_pattern.matcher(name);
+		if (!od_matcher.matches()){


need a space between ) and {

manhtran1609 · 2017-08-18T04:20:40Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+		if (!od_matcher.matches()){
+			LOG.debug("Pull request title " + branch_name  + " does not contain any ticket of BI project");
+		}
+		while (bi_matcher.find()){


need a space between ) and {

manhtran1609 · 2017-08-18T04:21:04Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+			String bi_id = bi_matcher.group().replaceAll("bi", jira_link + "bi-");
+			list_jira_link.add(bi_id);
+		}
+		if (!(list_jira_link.size()>0)){


need a space between ) and {

manhtran1609 · 2017-08-18T04:23:07Z

...blisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/api/GitHubApi.java

+   */
+  @Nullable
+  String getPullRequestTitle(@NotNull String repoOwner,
+                              @NotNull String repoName,


please line up the @NotNull vertically so it's easier to read

- Merge master into branch - Pass JIRA_LINK_CONF_NAME system parameter into script. Please add a new system parameter JIRA_LINK_CONF_NAME with value "https://ankiinc.atlassian.net/browse/" if using script GetJiraTickets.java - The script support for all of patterns of ticket id (od 123, od-123, od123) - The script works on the branch name

manhtran1609 · 2017-08-23T10:14:29Z

@lip please help me to review

sonnguyen-logigear · 2017-08-28T08:16:21Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+
+public class GetJiraTickets {
+  private static final Logger LOG = Logger.getInstance(GetJiraTickets.class.getName());
+  private static final String od = "od\\d*";


We should use upper case for constant

sonnguyen-logigear · 2017-08-28T08:51:03Z

...r-server/src/main/java/jetbrains/buildServer/commitPublisher/github/ChangeStatusUpdater.java

+                comment.append(ticketLink);
+                comment.append(")\n");
+              }
+            }


We should make this as a function.

sonnguyen-logigear · 2017-08-28T08:53:51Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+    while (odMatcher.find()) {
+      String odId = odMatcher.group().replaceAll("od", jiraLink + "od-");
+      listJiraLink.add(odId);
+    }


We should make it as a function, then reuse for cozmo, bi

sonnguyen-logigear · 2017-08-28T08:59:02Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+      String biId = biMatcher.group().replaceAll("bi", jiraLink + "bi-");
+      listJiraLink.add(biId);
+    }
+    if (!(listJiraLink.size()>0)) {


just check size == 0

sonnguyen-logigear · 2017-08-28T09:00:20Z

...erver/src/main/java/jetbrains/buildServer/commitPublisher/github/api/impl/GitHubApiImpl.java

+                                     @NotNull String repoName,
+                                     @NotNull String branchName) throws IOException {
+    final String pullRequestId = getPullRequestId(repoName, branchName);
+    if (pullRequestId == null) return null;


use if (condition) {
//code
} as standard

sonnguyen-logigear · 2017-08-28T09:01:29Z

...erver/src/main/java/jetbrains/buildServer/commitPublisher/github/api/impl/GitHubApiImpl.java

+
+    final RepoInfo head = pullRequestInfo.head;
+    if (head != null) {
+      return head.ref;


We should declare a variable to hold return value, and using return at the end of method only

sonnguyen-logigear · 2017-08-28T09:18:26Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+
+public class GetJiraTickets {
+  private static final Logger LOG = Logger.getInstance(GetJiraTickets.class.getName());
+  private static final String od = "od\\d*";


this pattern will match all candidate that contain od, we need a pattern to match od d+ or od-d+

sonnguyen-logigear · 2017-08-30T08:48:47Z

...r-server/src/main/java/jetbrains/buildServer/commitPublisher/github/ChangeStatusUpdater.java

+            for (int i = 0; i < listTicketLink.size(); i++) {
+              String ticketLink = listTicketLink.get(i);
+              String ticketId = ticketLink.substring(ticketLink.lastIndexOf("/") + 1).toUpperCase();
+              String commentLine = "[" + ticketId + "](" + ticketLink + ")\n";


I think it should a space instead of "\n" as : OD-1 OD-2

sonnguyen-logigear · 2017-08-30T09:08:29Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+  public static List<String> getListTicketLink(@NotNull String branchName, 
+                                               @NotNull String jiraLink) {
+
+    String name = branchName.toLowerCase().replaceAll("[^\\da-z]", "");


We only support patterns : od-123 and od 123, no need to replaceAll here

sonnguyen-logigear · 2017-08-30T09:27:10Z

...lisher-server/src/main/java/jetbrains/buildServer/commitPublisher/github/GetJiraTickets.java

+    }
+    if (listTicketLink.size() == 0) {
+      LOG.debug("Pull request title " + branchName + " does not contain any ticket id");
+      listTicketLink = null;


We no need to assign listTicketLink to null, we can check by get size of it.

sonnguyen-logigear · 2017-09-05T08:45:02Z

...r-server/src/main/java/jetbrains/buildServer/commitPublisher/github/ChangeStatusUpdater.java

+
+          private StringBuilder getJiraTicketComment(@NotNull List<String> listTicketLink) {
+            StringBuilder jiraLinksComment = new StringBuilder();
+            for (int i = 0; i < listTicketLink.size(); i++) {


using for (String ticketLink : listTicketLink) as our standard

sonnguyen-logigear · 2017-09-05T09:04:11Z

...r-server/src/main/java/jetbrains/buildServer/commitPublisher/github/ChangeStatusUpdater.java

+            final String jiraLink = getJiraLink(build);
+            final List<String> listTicketLink = GetJiraTickets.getListTicketLink(refBranch, jiraLink);
+            if (listTicketLink.size() != 0) {
+              StringBuilder ticketLinkComment = getJiraTicketComment(listTicketLink);


We just need to you String instead of StringBuilder

tungledao added 2 commits August 9, 2017 15:03

BI-168-Add-a-way-to-get-to-Jira-tickets-from-github

a2c4d3d

Merge from master

60b7a0d

tungledao requested review from manhtran1609 and sonnguyen-logigear August 9, 2017 09:41

Comment will show only bug ID

3ebf782

manhtran1609 requested changes Aug 18, 2017

View reviewed changes

tungledao added 3 commits August 22, 2017 17:29

Update variables

67aa7e6

Update GetJiraTickets.java

54adb82

manhtran1609 approved these changes Aug 23, 2017

View reviewed changes

sonnguyen-logigear reviewed Aug 28, 2017

View reviewed changes

tungledao added 2 commits August 29, 2017 11:37

Update BI-168

a423670

Update to support for many id tickets

7ec8e51

sonnguyen-logigear reviewed Aug 30, 2017

View reviewed changes

Update Pattern

e3b79c6

sonnguyen-logigear reviewed Sep 5, 2017

View reviewed changes

Update to foreach

c57f5b9

Bi 168 add a way to get to jira tickets from GitHub #8

Are you sure you want to change the base?

Bi 168 add a way to get to jira tickets from GitHub #8

Uh oh!

Conversation

tungledao commented Aug 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manhtran1609 commented Aug 23, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonnguyen-logigear Aug 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonnguyen-logigear Aug 30, 2017 •

edited

Loading