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

Reduce use of naked returns #162

Merged
merged 1 commit into from
May 8, 2015
Merged

Conversation

rolandshoemaker
Copy link
Contributor

or _The great enrobing!_

Fixes for #152 as well as various return related cleanups, I've left naked returns on most smaller functions (~ <30 lines) where readability doesn't really suffer too much.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 47.15% when pulling ee47c84 on rolandshoemaker:enrobe into f6c7b49 on letsencrypt:master.

@@ -62,13 +62,15 @@ type CertificateAuthorityImpl struct {
// using CFSSL's authenticated signature scheme. A CA created in this way
// issues for a single profile on the remote signer, which is indicated
// by name in this constructor.
func NewCertificateAuthorityImpl(cadb core.CertificateAuthorityDatabase, config Config) (ca *CertificateAuthorityImpl, err error) {
func NewCertificateAuthorityImpl(cadb core.CertificateAuthorityDatabase, config Config) (*CertificateAuthorityImpl, error) {
var ca *CertificateAuthorityImpl
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's okay, even if you're using clothed returns, to declare the variables in the return statement of the function. You would just name them explicitly in the return. But I'm not 100% on go standard style, so feel free to correct me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, it seems like this is the only spot where this comment is relevant, so I'm not going to worry about it.

Thanks for this change, nice improvement!

jsha added a commit that referenced this pull request May 8, 2015
@jsha jsha merged commit 14fde00 into letsencrypt:master May 8, 2015
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

3 participants