Skip to content

Conversation

@rehandalal
Copy link
Contributor

f?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do this, it will give a 50x error if the user doesn't pass a tag. It is better to do something like in delete_metadata to check that it is the data, and if not, give a 400 bad request error. 500s are bad because the are completely opaque to users.

@mythmon
Copy link
Contributor

mythmon commented Feb 11, 2015

This is looking good. I made some comments that are mostly about friendliness of the API, and consistency. I didn't test this though.

@rehandalal
Copy link
Contributor Author

Ready to go, with tests.

r?

@rehandalal rehandalal changed the title (WIP) [bug 1128477] Tags API for questions [bug 1128477] Tags API for questions Feb 12, 2015
@mythmon
Copy link
Contributor

mythmon commented Feb 12, 2015

r+, but this seems to have a merge conflict. Want to fix that, and merge this?

rehandalal added a commit that referenced this pull request Feb 12, 2015
[bug 1128477] Tags API for questions
@rehandalal rehandalal merged commit 2b363af into mozilla:master Feb 12, 2015
@rehandalal rehandalal deleted the tags-api branch February 16, 2015 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants