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

Use own toLowerCamel for arguments since ID must be id not iD. #1

Merged
merged 1 commit into from
Feb 25, 2020

Conversation

mattn
Copy link
Contributor

@mattn mattn commented Dec 10, 2019

A consecutive words containing common words sometimes have to be converted with own case independently.

@mattn mattn changed the title Use own toLowerCamel for arguments since ID must be id. Use own toLowerCamel for arguments since ID must be id not iD. Dec 10, 2019
@mattn
Copy link
Contributor Author

mattn commented Dec 10, 2019

Please take a look carefully since I'm not sure this toLowerCase support all of case we want.

@moznion moznion self-requested a review December 10, 2019 01:49
@moznion
Copy link
Owner

moznion commented Dec 10, 2019

Thank you for contributing.

The idea seems really good. However, I'm considering whether this library should have such function inside.
I suppose there are following three ways to format the string case precisely:

  • merge this pull-request
  • extract changes of this pull-request to an external package and use it
  • use golint output
    • IIRC, golint shows a warning like: main.go:5:6: func useHttp should be useHTTP
    • if could, how about reusing golint's code?

What do you think? Just for the record, I think this pull-request looks good to me!

@moznion moznion added the enhancement New feature or request label Dec 10, 2019
@moznion
Copy link
Owner

moznion commented Dec 10, 2019

If we could use golint auto-correction, it would be easy to support...
golang/lint#214

@mattn
Copy link
Contributor Author

mattn commented Dec 10, 2019

Thanks your pointing the URL. I'll look into it in later. 👍

@moznion
Copy link
Owner

moznion commented Feb 25, 2020

Hello

I've implemented the go-lint fixing feature for this purpose, but I'm not sure when will it be merged (possibly never): golang/lint#476
So now, I'm welcome to merge this commit. I'll do that.

@kovetskiy
By the way, are you interested in to be a co-maintainer of this project? If so, I'm very happy.

@kovetskiy
Copy link
Collaborator

Hi,

By the way, are you interested in to be a co-maintainer of this project? If so, I'm very happy.
@moznion sure, it will be much easier.

@moznion moznion merged commit a2173d9 into moznion:master Feb 25, 2020
@moznion
Copy link
Owner

moznion commented Feb 25, 2020

@mattn thank you for your pull-request! I've just merged that with fixing conflicts.

@moznion
Copy link
Owner

moznion commented Feb 25, 2020

@kovetskiy thank you for your cooperation. I've just sent an invitation for that.
And I'm reviewing #3 now, I'm sorry for the delayed response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants