Refactor and decouple REST api framework

[dbrenden]

The recording API endpoints have been moved from the server.py module to a new class called recording.py.

[dideler]

Great, can you just move the reference to 568 from the title to the PR description? Please put it at the bottom of the description as "Closes #568".

Edit: It's common to use imperative style when using Git. So "Moved recording ..." would be "Move recording ...". There's lots of resources online explaining why this is preferred, if you're interested in learning more.

[zxiiro]

Regarding the adding "Closes #568" to the description. You should actually add it to your commit message so that it is linked in the commit message back to the issue.

Your commit message should look something like

Moved recording API endpoints from server.py to recording.py
Some detailed description if neccessary
Closes #568

[dideler]

You should actually add it to your commit

Yeah, since these commits are squashed that would work (I don't always squash my commits, which is why I rather close it from the PR). Having the reference in both places (i.e. PR description and commit message) would be most convenient, and is simple enough to do.

[mtomwing]

FYI, the build failed because of some flake8 errors:

@RUNNING: flake8 src
src/freeseer/frontend/controller/__init__.py:3:1: F401 'recording' imported but unused
src/freeseer/frontend/controller/recording.py:200:1: W391 blank line at end of file
src/freeseer/frontend/controller/server.py:90:27: F821 undefined name 'ServerError'
@EXIT WITH STATUS: 1
@FAILING...

[mtomwing]

Looks good. Though there is still room for more refactoring. Like for example since all the endpoints return JSON, it would be nice if there was a decorator that did the transformation automatically and then the endpoint function could simply return a dict.

[mtomwing]

This needs to also return a response, but with the HTTPError's status code and message.

Edit: Unless Flask already does that when you return a tuple?

[mtomwing]

Looks like this might not be needed anymore.

[mtomwing]

On IRC you mentioned that you fixed the last few things I mentioned, but I don't see any changes.

[mtomwing]

Assuming this is already rebased against master, please squash it and I'll do the merge later today.

[dideler]

@dbrenden Michael's afk right now, but he told me over Hangouts that he wants you to squash it into a single commit since it's all refactoring work.

[mtomwing]

Once the commit message is fixed up I'll merge this. As mentioned on IRC I want a description of what happened, not a list of every single change.

[mtomwing]

Thanks for your contribution!

[dideler]

Looking at the comments, this was originally supposed to close #568 but it never did. @dbrenden what's the status of #568?

[dbrenden]

The work for #568 is done so it should be closed.

On 21 Oct 2014 07:43, "Dennis Ideler" notifications@github.com wrote:

Looking at the comments, this was originally supposed to close #568 but
it never did. @dbrenden what's the status of #568?

—
Reply to this email directly or view it on GitHub.