Summary

Make the jenkins plugin notification a little nicer.

Review Request #10384 — Created Jan. 15, 2019 and submitted Jan. 15, 2019, 5:10 p.m.

Information

Owner

david

Repository

Jenkins plugin

Branch

master

Bugs

Depends On

Commit

85c1083...

Reviewers

Groups

jenkins-plugin

People

Description

The jenkins plugin was initially written to update the status update on
the review request once the build completed (either successfully or
not). This change enhances the notification in several ways:

We now post a change to the status update when initially applying the
patch. This will change the status from "starting build" (assigned by
the integration) to "build running".
Add a link to the in-progress build.
Fix the post-build notification to properly use the error state
instead of failure, when the build failed because of some non-code
related issue.

Testing Done


Ran plugin unit tests.
Built and loaded the plugin into my Jenkins instance.
Posted a change to a review request. Saw that the status soon changed

  to "build running" with a correct link to the build. Saw that the

  build finished successfully.

Issues

Description	From	Last Updated
Blank line between these.	chipx86	Jan. 15, 2019, 5:01 p.m.
How about using the format we use for querysets here? final ReviewBoardGlobalConfiguration globalConfig = ( GlobalConfiguration.all() .get(ReviewBoardGlobalConfiguration.class) ); Same below.	chipx86	Jan. 15, 2019, 5:01 p.m.
Blank line between these.	chipx86	Jan. 15, 2019, 5:02 p.m.
Blank line between these.	chipx86	Jan. 15, 2019, 5:02 p.m.
No space after the cast.	chipx86	Jan. 15, 2019, 5:02 p.m.
Can we make this wrap to 80 chars?	chipx86	Jan. 15, 2019, 5:03 p.m.
Can we have a blank line between the cases, so it's easier to read?	chipx86	Jan. 15, 2019, 5:03 p.m.

flake8 passed.

JSHint passed.

Basing the style-related comments on how we handle things in the rest of our codebases (and from what I recall being common back when I used to write Java code).

src/main/java/org/reviewboard/rbjenkins/common/ReviewBoardUtils.java (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line between these.
```

src/main/java/org/reviewboard/rbjenkins/common/ReviewBoardUtils.java (Diff revision 1)

The issue has been resolved. Show all issues

How about using the format we use for querysets here?

final ReviewBoardGlobalConfiguration globalConfig = (
    GlobalConfiguration.all()
    .get(ReviewBoardGlobalConfiguration.class)
);


Same below.

src/main/java/org/reviewboard/rbjenkins/common/ReviewBoardUtils.java (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line between these.
```
src/main/java/org/reviewboard/rbjenkins/common/ReviewBoardUtils.java (Diff revision 1)
The issue has been resolved. Show all issues
```
Blank line between these.
```
src/main/java/org/reviewboard/rbjenkins/common/ReviewBoardUtils.java (Diff revision 1)
The issue has been resolved. Show all issues
```
No space after the cast.
```
src/main/java/org/reviewboard/rbjenkins/common/ReviewBoardUtils.java (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we make this wrap to 80 chars?
```

src/main/java/org/reviewboard/rbjenkins/common/ReviewBoardUtils.java (Diff revision 1)

The issue has been resolved. Show all issues

Can we have a blank line between the cases, so it's easier to read?

Commit:

696f5b808bedb4ca0ca0b1f19057becaa13d8d6e

85c108390fe89c38ec43d453223213eaca262431

Diff:

Revision 2 (+159 -111)

Show changes

	src/main/java/org/reviewboard/rbjenkins/common/ReviewBoardUtils.java
	src/main/java/org/reviewboard/rbjenkins/steps/ReviewBoardNotifier.java
	src/main/java/org/reviewboard/rbjenkins/steps/ReviewBoardSetup.java
	src/test/java/org/reviewboard/rbjenkins/steps/ReviewBoardNotifierTest.java

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (85c1083)