Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Allow Resource.Fingerprint to be the zero value. #189
Conversation
ericsnowcurrently
added some commits
Jan 12, 2016
natefinch
reviewed
Jan 12, 2016
| - return errors.Annotate(err, "bad fingerprint") | ||
| + if res.Fingerprint.IsZero() { | ||
| + if res.Size > 0 { | ||
| + return errors.NewNotValid(nil, "missing fingerprint; sized resources must have one") |
natefinch
Jan 12, 2016
Contributor
error text is kind of awkward. 0 is a size ;)
I think it's sufficient to say "missing fingerprint".
natefinch
reviewed
Jan 12, 2016
| @@ -47,11 +47,11 @@ func ParseFingerprint(raw string) (Fingerprint, error) { | ||
| } | ||
| // GenerateFingerprint returns the fingerprint for the provided data. | ||
| -func GenerateFingerprint(data io.Reader) (Fingerprint, error) { | ||
| +func GenerateFingerprint(reader io.Reader) (Fingerprint, error) { |
|
I would add a test for GenerateFingerprint(nil), and ensure it validates correctly. |
|
Otherwise, LGTM. |
ericsnowcurrently
added some commits
Jan 12, 2016
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju-charm |
|
Build failed: Tests failed |
|
$$try-again$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju-charm |
added a commit
that referenced
this pull request
Jan 12, 2016
jujubot
merged commit 5f443e0
into
juju:v6-unstable
Jan 12, 2016
ericsnowcurrently
deleted the
ericsnowcurrently:resources-fingerprint-zero-value
branch
Jan 12, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ericsnowcurrently commentedJan 12, 2016
No description provided.