Reviews
Diff

- Download Diff

Summary:

Add a resource for retrieving a list of commits

Review Request #8922 — Created May 5, 2017 and submitted May 23, 2017, 3:15 p.m.

Information
Owner:	brennie
Repository:	gerrit-reviewboard-plugin
Branch:	master
Bugs:
Depends On:	~~8920~~
Blocks:	8923
Reviewers
Groups:	gerrit-reviewboard-plugin
People:

Description

The CommitListResource, available at
a/projects/<projectName>/all-commits/, returns a list of commit
information entries about the first N (default 30) commits on a
branch. It also supports starting at a specific commit for
pagination support.

Testing Done

Hit the API with ?limit and ?start queries and saw the correct
results.

Issues

Description	From	Last Updated
Inconsistent spacing around =s. Shouldn't have any spaces, I think.	david	May 11, 2017, 8:58 a.m.
Inconsistent spacing around =s. Shouldn't have any spaces, I think. Also, CNT?	david	May 11, 2017, 8:58 a.m.
Should we be doing some kind of logging rather than just saying to print to console?	david	May 11, 2017, 8:58 a.m.
What happens if the commit has no parents?	david	May 22, 2017, 10:45 p.m.
Wrap this?	david	May 11, 2017, 8:58 a.m.

JSHint passed.

PEP8 Style Checker passed.

Pyflakes passed.

Diff:

Revision 2 (+158)

	src/main/java/org/reviewboard/rbgerrit/CommitListResource.java
	src/main/java/org/reviewboard/rbgerrit/Module.java

Checks run (3 succeeded)

JSHint passed.

PEP8 Style Checker passed.

Pyflakes passed.

Change Summary:


Move the resource under the project
add full message
add author time

Description:

		The `CommitListResource`, available at
~		`a/projects/<projectName>/branches/<branchName>/commits/`, returns a
~		list of commit information entries about the first N (default 30)
~		commits on a branch. It also supports starting at a specific commit
~		for pagination support.
	~	`a/projects/<projectName>/all-commits/`, returns a list of commit
	~	information entries about the first N (default 30) commits on a
	~	branch. It also supports starting at a specific commit for
	~	pagination support.

Diff:

Revision 3 (+163)

	src/main/java/org/reviewboard/rbgerrit/CommitListResource.java
	src/main/java/org/reviewboard/rbgerrit/Module.java

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Diff:

Revision 4 (+165)

	src/main/java/org/reviewboard/rbgerrit/CommitListResource.java
	src/main/java/org/reviewboard/rbgerrit/Module.java

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

src/main/java/org/reviewboard/rbgerrit/CommitListResource.java (Diff revision 4)
Show all issues
```
Inconsistent spacing around =s. Shouldn't have any spaces, I think.
```
src/main/java/org/reviewboard/rbgerrit/CommitListResource.java (Diff revision 4)
Show all issues
```
Inconsistent spacing around =s. Shouldn't have any spaces, I think.
Also, CNT?
```
1. brennie May 11, 2017, 8:58 a.m.
  It was short for COUNT. :/
src/main/java/org/reviewboard/rbgerrit/CommitListResource.java (Diff revision 4)
Show all issues
```
Should we be doing some kind of logging rather than just saying to print to console?
```

src/main/java/org/reviewboard/rbgerrit/CommitListResource.java (Diff revision 4)

Show all issues

What happens if the commit has no parents?

brennie May 11, 2017, 8:49 a.m.

The walk that generates these commits (see line 94, 139--) ensures that each commit has exactly a single parent.

david May 11, 2017, 9:56 a.m.

In that case, rather than having a conditional, can we assert?

david May 19, 2017, 12:01 p.m.
```
Ping on this?
```

src/main/java/org/reviewboard/rbgerrit/CommitListResource.java (Diff revision 4)
Show all issues
```
Wrap this?
```

Diff:

Revision 5 (+167)

	src/main/java/org/reviewboard/rbgerrit/CommitListResource.java
	src/main/java/org/reviewboard/rbgerrit/Module.java

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

assert over if.

Diff:

Revision 6 (+166)

	src/main/java/org/reviewboard/rbgerrit/CommitListResource.java
	src/main/java/org/reviewboard/rbgerrit/Module.java

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Loading...