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

Fixes #5545 #5590

Merged
merged 2 commits into from
Feb 10, 2016
Merged

Fixes #5545 #5590

merged 2 commits into from
Feb 10, 2016

Conversation

e-dard
Copy link
Contributor

@e-dard e-dard commented Feb 9, 2016

Refactor out the SUBSCRIPTION tests for some better coverage and clarity. Ensure that a retention policy error will be returned if you attempt to drop a subscription on a policy that doesn't exist.

Removed meta.ErrRetentionPolicyNotFound since everything else uses influxdb. ErrRetentionPolicyNotFound()

@e-dard e-dard changed the title Fixes 5545 Fixes #5545 Feb 9, 2016
@jwilder jwilder mentioned this pull request Feb 9, 2016
@seiflotfy
Copy link
Contributor

Tested and it works 👍

@@ -574,8 +574,7 @@ func (data *Data) CreateSubscription(database, rp, name, mode string, destinatio
rpi, err := data.RetentionPolicy(database, rp)
if err != nil {
return err
}
if rpi == nil {
} else if rpi == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think golint prefers not having the else as it was previously written.

@corylanou
Copy link
Contributor

minor nit (that I'm not 100% sure on). +1

@e-dard
Copy link
Contributor Author

e-dard commented Feb 9, 2016

@corylanou golint seems fine with it, though there were some pre-existing lint issues in files I touched in this PR, so I've tidied those up.

@corylanou
Copy link
Contributor

+1

@jwilder
Copy link
Contributor

jwilder commented Feb 9, 2016

👍

@jwilder jwilder added this to the 0.11.0 milestone Feb 9, 2016
e-dard added a commit that referenced this pull request Feb 10, 2016
@e-dard e-dard merged commit bb9eed9 into master Feb 10, 2016
@e-dard e-dard deleted the er-sub-panic branch February 10, 2016 10:46
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.

4 participants