This is an automatically generated e-mail. To reply, visit: http://reviewboard-tflink.rhcloud.com/r/40/

blockerbugs.spec (Diff revision 2)
24
Required:       python-iso8601
'Required' is not a valid keyword for spec files, it's 'Requires'. Also, if iso8601 is used during unit tests, you'll have to add a 'BuildRequires'. Have you built an RPM with this spec?

blockerbugs/controllers/api/api.py (Diff revision 2)
21
#   Tim Flink <tflink@redhat.com>
You're the author here, not me :)

There are several cases of this in this patch. I'm just marking one, though.

blockerbugs/controllers/api/api.py (Diff revision 2)
153
@api_v0.route('/milestone/<int:rel_num>/<rel_name>/update')
out of curiosity, how come you're using <rel_name> here instead of something like milestone?

blockerbugs/controllers/api/api.py (Diff revision 2)
189
@api_v0.route('/milestone/<int:rel_num>/<rel_name>/spin')
One of the things that I'm wondering about is consistency with the existing pages and whether or not this is important.

Specifically, I'm questioning naming this route 'spin' when the gui uses 'spins' and whether this would count as an inconsistency for API discovery - thoughts?

I'm also wondering if it's useful enough to allow GET on milestone/<rel_num>/<rel_name>/spin/<id> to get one spin instead of all spins for a release/milestone

blockerbugs/controllers/api/api.py (Diff revision 2)
198
@api_v0.route('/milestone/<int:rel_num>/<rel_name>/spin', methods=['POST'])
it seems to me that this POST might be simpler (ie, less validation) if the URI had an <id> at the end - do you think that would add or take away any utility of the API?

blockerbugs/controllers/api/api.py (Diff revision 2)
222
        spin_kwargs['date_requested'] = parse_date(data['date_requested'])
I think this should be calculated at request time instead of just parsing what the client sends in - the request time should be pretty much when the request is received, anyways

It's looking pretty good - I think you're planning to add a method for spin modification as we talked about on IRC which is one of my biggest concerns right now.

- Tim Flink


On August 5th, 2013, 10:15 a.m. UTC, Ilgiz Islamgulov wrote:

Review request for blockerbugs.
By Ilgiz Islamgulov.

Updated Aug. 5, 2013, 10:15 a.m.

Bugs: 392
Repository: blockerbugs

Description

Add endpoints:
 - list bugs
 - list updates
 - list spins
 - create spin

Testing

Wrote test suites.
I've tested on my develop instance.

Diffs

  • testing/test_validators.py (PRE-CREATION)
  • testing/test_api.py (PRE-CREATION)
  • requirements.txt (09e0318bc189512f5d324bda8879ad74c4763f95)
  • blockerbugs/models/spin.py (99891448e78c7168d540b479ffd7ef00ce1eec1d)
  • blockerbugs/controllers/api/validators.py (PRE-CREATION)
  • blockerbugs/controllers/api/errors.py (PRE-CREATION)
  • blockerbugs/controllers/api/api.py (PRE-CREATION)
  • blockerbugs/controllers/api/__init__.py (PRE-CREATION)
  • blockerbugs/__init__.py (c4f2c34f5eac713253336d026aa9485259096cb0)
  • blockerbugs.spec (726fa6920c67cfe36a2e544f97c9e0f16537f11e)

View Diff