Where would branchname come from?
I was wondering if we could reuse the pull_request template here instead of having this large if block in the repo_info.
pull_request
if
I didn't investigate, just thought :)
I've been slowly moving from assertTrue to assertIn but it's up to you which one you like best.
assertTrue
assertIn
We don't really need a branch do we?
branchname and branch: Oops. I removed the branch things but left that behind. Will remove it in a fix commit.
The assertIn, ok. Will change it too. It seems more accurate :)
Will investigate the pull_request template to check if could be used instead that if block
Pull-Request has been updated
Finally I used the pull request template as you said. It was easier to adapt it than I imagined. :)
I suspect this will not please pep8, could you see about splitting this over 2 lines?
(Similar for the lines below when they are more than 80 characters)
So I've come to realize that having URLs of type / isn't a good idea, especially if we want to work on things like namespace of pseudo-namespace.
Since this is a new URL endpoint, let's try to get it right from the start, I propose: /<repo>/c/<commit1>..<commit2>/
/<repo>/c/<commit1>..<commit2>/
Thoughts?
Sorry for the late review, I had missed your last comment, but kudos, looks much better this way :)
The PEP8 styling has been done. I've the line length limit warning disabled in the editor and forgot to check it.
I agree with the /<repo>/c/<commit1>..<commit2>/ schema. Will change it as soon as you confirm.
Thanks :)
I agree with the //c/../ schema. Will change it as soon as you confirm.
I confirm :) (Even more, check the namespace branch ;-))
Have checked it, but don't know if you want me to do something with that. I've seen you reworked a lot of routes as well as some of the views.
Will push the route modifications and wait for your response.
Oh, since this is a new endpoint, no need to enforce a size for the commit hash.
I did this in namespace to keep backward compatibility, but if you check the endpoint for /<repo>/v/<commit> it doesn't enforce a commit length, allowing to use shorter hash.
/<repo>/v/<commit>
Ok. Removing! :)
Looks good to me.
Could you rebase your branch? I'll test the change locally and if all goes well merge :)
Pull-Request has been closed by ogutierrez