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

Fixed typos and metrics name #233

Merged
merged 1 commit into from
Sep 21, 2018
Merged

Fixed typos and metrics name #233

merged 1 commit into from
Sep 21, 2018

Conversation

chebyrash
Copy link
Contributor

I have fixed a couple of typos in comments, READMEs and variable names. However, there was typo in a the following line of code - ...WithLabelValues(h.protocol, "unsibscribe")... where "unsubscribe" was incorrectly spelled. This can be a problem if metrics label names are incorrectly spelled, so it might be a good idea to put those names in a separate const ( ... ) space.

@FZambia
Copy link
Member

FZambia commented Sep 21, 2018

Thanks! I've also pushed a fix for CI to not try login to docker on pull requests in fa01b4b.

@FZambia FZambia merged commit 92849f4 into centrifugal:master Sep 21, 2018
FZambia pushed a commit that referenced this pull request Sep 21, 2018
@FZambia
Copy link
Member

FZambia commented Sep 21, 2018

@chebyrash also I returned several lines you deleted for some reason: e864024

@chebyrash
Copy link
Contributor Author

@FZambia sorry, I deleted the wrong lines. I wanted to fix it like below
Golint shows that code from e864024

if err = addExtensionConventionForRollups(buf, mf, m); err != nil {
		return err
	}
return nil

Can be rewritten because if ...; err != nil check, just return error instead.

return addExtensionConventionForRollups(buf, mf, m);

@FZambia
Copy link
Member

FZambia commented Sep 21, 2018

Yeah, right – if you have time please send another pr with this fix, if not – I'll fix it myself soon.

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.

2 participants