Support the new API workflow for posting commit series

Review Request #10240 — Created Oct. 18, 2018 and submitted

Information

RBTools
master
63df254...

Reviewers

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.

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

Description From Last Updated

Leftover debug output?

daviddavid

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

chipx86chipx86

Same here.

chipx86chipx86

Should this be bytes?

chipx86chipx86

This should be a Unicode string.

chipx86chipx86

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

chipx86chipx86
brennie
david
  1. 
      
  2. rbtools/commands/post.py (Diff revision 2)
     
     

    Leftover debug output?

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

    2. 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.

  3. 
      
brennie
chipx86
  1. 
      
  2. rbtools/api/resource.py (Diff revision 3)
     
     
     

    Can you add something like:

    if isinstance(cumulative_diff, bytes):
        raise TypeError('cumulative_diff must be a byte string, not %s' % type(cumulative_diff))
    
  3. rbtools/api/resource.py (Diff revision 3)
     
     

    Same here.

  4. rbtools/commands/post.py (Diff revision 3)
     
     

    Should this be bytes?

  5. 
      
brennie
chipx86
  1. 
      
  2. rbtools/api/resource.py (Diff revision 4)
     
     

    This should be a Unicode string.

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

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

  4. 
      
brennie
chipx86
  1. Ship It!
  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (400e77a)
Loading...