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

multi: Error descriptions should be lowercase. #771

Merged
merged 1 commit into from
Aug 15, 2017

Conversation

atriviss
Copy link
Contributor

As per https://github.com/golang/go/wiki/CodeReviewComments#error-strings,
error strings should not be capitalized

  • Replace Capitalized by lower cases
  • Also, at some places, made a judgement call and added a prefix

As per https://github.com/golang/go/wiki/CodeReviewComments#error-strings,
error strings should not be capitalized
 Replace Capitalized by lower cases
 Also, at some places, made a judgement call and added a prefix
@davecgh
Copy link
Member

davecgh commented Jul 27, 2017

Thanks for the PR. I haven't reviewed this yet, but the first thing I noticed is the commit will need to start with the prefix multi: since it touches multiple packages and the issue number must not be in the commits in the repository, rather any issue numbers should be referenced in the PR description.

Commits are often shared between repositories, so referencing a specific repository's github-specific numbers in commits causes problems where it tags the wrong issues/PRs and otherwise generally causes confusion.

@davecgh
Copy link
Member

davecgh commented Jul 29, 2017

Code wise this looks fine. However, as mentioned above, the commit title will need to modified per the above and the PR updated with it before it can be merged.

@davecgh davecgh changed the title Issue 363: Error descriptions should be lowercase multi: Error descriptions should be lowercase. Aug 15, 2017
@davecgh davecgh merged commit ae973eb into decred:master Aug 15, 2017
@davecgh
Copy link
Member

davecgh commented Aug 15, 2017

For reference, I used squash to correct the commit title since the original author never followed up.

@energiplatform
Copy link

thanks

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.

3 participants