Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the archive delete API endpoint. #80

Merged
merged 4 commits into from Aug 25, 2014
Merged

Conversation

frankban
Copy link
Member

Also fixed tests not properly skipped in the case Mongo
has no JS support, and improved the test separation
when counters are exercised.

Also fixed tests not properly skipped in the case Mongo
has no JS support, and improved the test separation
when counters are exercised.
@jujugui
Copy link
Contributor

jujugui commented Aug 25, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/charmstore/149/

@jujugui
Copy link
Contributor

jujugui commented Aug 25, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/charmstore/150/

if err := h.store.BlobStore.Remove(blobName); err != nil {
return errgo.Notef(err, "cannot remove blob %s", blobName)
}
// TODO frankban 2014-08-25: log possible IncCounter errors.
Copy link

Choose a reason for hiding this comment

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

Didn't we agree TODO needed bug numbers when reviewed? Or was that just XXX?

Copy link
Member Author

Choose a reason for hiding this comment

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

Uros is adding a card on the board.
Logging is a feature still under discussion.
Thanks for the good catch.

@bac
Copy link

bac commented Aug 25, 2014

👍

1 similar comment
@jrwren
Copy link
Contributor

jrwren commented Aug 25, 2014

👍

@@ -35,6 +35,8 @@ func (h *handler) serveArchive(id *charm.Reference, w http.ResponseWriter, req *
default:
// TODO(rog) params.ErrMethodNotAllowed
return errgo.Newf("method not allowed")
case "DELETE":
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the link to the doc API for delete implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, done.

@urosj
Copy link
Contributor

urosj commented Aug 25, 2014

With comments, otherwise 👍

@frankban
Copy link
Member Author

Thanks for the reviews!
:shipit:

@jujugui
Copy link
Contributor

jujugui commented Aug 25, 2014

Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/charmstore-merge

@jujugui
Copy link
Contributor

jujugui commented Aug 25, 2014

Test PASSed.
Refer to this link for build results: http://ci.jujugui.org:8080/job/charmstore/151/

jujugui added a commit that referenced this pull request Aug 25, 2014
Implement the archive delete API endpoint.

Also fixed tests not properly skipped in the case Mongo
has no JS support, and improved the test separation
when counters are exercised.
@jujugui jujugui merged commit 9d944c0 into juju:v4 Aug 25, 2014
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.

None yet

5 participants