Summary

Support the new API workflow for posting commit series

Review Request #10240 — Created Oct. 18, 2018 and submitted Oct. 25, 2018, 10:58 p.m.

Information

Owner

brennie

Repository

RBTools

Branch

master

Bugs

Depends On

~~10233~~, ~~9921~~

Commit

63df254...

Reviewers

Groups

rbtools

People

Description

RBTools now uploads a cumulative diff of the entire commit series after
all commits have been uploaded so (a) Review Board can have a copy of
this diff instead of generating it and (b) Review Board can validate
the uploaded commits fully represent the range uploaded.

Testing Done

Posted a commit series for review and then published it. It worked!

Issues

Description	From	Last Updated
Leftover debug output?	david	Oct. 23, 2018, 5:59 p.m.
Can you add something like: if isinstance(cumulative_diff, bytes): raise TypeError('cumulative_diff must be a byte string, not %s' % type(cumulative_diff))	chipx86	Oct. 25, 2018, 9:52 p.m.
Same here.	chipx86	Oct. 25, 2018, 9:53 p.m.
Should this be bytes?	chipx86	Oct. 25, 2018, 9:53 p.m.
This should be a Unicode string.	chipx86	Oct. 25, 2018, 10:50 p.m.
Parameter validation should be done at the top of the function, before we do anything else.	chipx86	Oct. 25, 2018, 10:50 p.m.

flake8 passed.

JSHint passed.

Commit:

5f4e3845a2dfbaa82cd8a3deac4dc1a48f8668a8

0c85783c086e2b3754f24dc476c52ed31d0a86b5

Diff:

Revision 2 (+58 -15)

Show changes

	rbtools/api/resource.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/commands/post.py (Diff revision 2)

The issue has been resolved. Show all issues

Leftover debug output?

brennie Oct. 23, 2018, 2:24 p.m.

No, I included this on purpose. Should I remove it?

david Oct. 23, 2018, 2:26 p.m.

This is super programmer-speak. I could see maybe "Finishing..." but assuming it's not a super long-running operation I'd rather just remove it.

Change Summary:

Addressed feedback

Commit:

0c85783c086e2b3754f24dc476c52ed31d0a86b5

213b773602c4d66a4ad98ecaadf19dc1e16bbe67

Diff:

Revision 3 (+57 -15)

Show changes

	rbtools/api/resource.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/api/resource.py (Diff revision 3)

The issue has been resolved. Show all issues

Can you add something like:

if isinstance(cumulative_diff, bytes):
    raise TypeError('cumulative_diff must be a byte string, not %s' % type(cumulative_diff))

rbtools/api/resource.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Same here.
```
rbtools/commands/post.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Should this be bytes?
```

Change Summary:

Addressed feedback.

Commit:

213b773602c4d66a4ad98ecaadf19dc1e16bbe67

5daf19c2446322905b7bf2839edfd8887ea3dd83

Diff:

Revision 4 (+65 -15)

Show changes

	rbtools/api/resource.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/api/resource.py (Diff revision 4)
The issue has been resolved. Show all issues
```
This should be a Unicode string.
```

rbtools/api/resource.py (Diff revision 4)

The issue has been resolved. Show all issues

Parameter validation should be done at the top of the function, before we do anything else.

Change Summary:

Addressed feedback

Commit:

5daf19c2446322905b7bf2839edfd8887ea3dd83

63df254eff07108954c6a0c892a1672b8e8e8d39

Diff:

Revision 5 (+65 -15)

Show changes

	rbtools/api/resource.py
	rbtools/commands/post.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (400e77a)