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 the more conventional camel case for generated parameters #59

Merged
merged 17 commits into from
Jul 25, 2019

Conversation

atifaziz
Copy link
Contributor

This PR fixes issue #57.

Copy link
Owner

@amis92 amis92 left a comment

Choose a reason for hiding this comment

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

Also, please add Changelog entry :)

@atifaziz
Copy link
Contributor Author

Also, please add Changelog entry :)

Done with 5506650.

Copy link
Owner

@amis92 amis92 left a comment

Choose a reason for hiding this comment

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

Shouldn't With-methods use camelCase as well?

[xUnit.net 00:00:00.44] Amadevus.RecordGenerator.Test:
    Skipping test case with duplicate ID f4964700a5ae6318d746f8f9e7f17b599fcfdcaf:
        Parameter_Name_Uses_Camel_Case(type: "Builder", method: "set_Id", name: "value")
        Parameter_Name_Uses_Camel_Case(type: "Builder", method: "set_Id", name: "value")
[xUnit.net 00:00:00.44] Amadevus.RecordGenerator.Test:
    Skipping test case with duplicate ID '3bc3e875c6bc477ff0e62177a965bccb12311bc1:
        Parameter_Name_Uses_Camel_Case(type: "Builder", method: "set_Name", name: "value")
        Parameter_Name_Uses_Camel_Case(type: "Builder", method: "set_Name", name: "value")
@amis92
Copy link
Owner

amis92 commented Jul 25, 2019

I'm in the middle of changing CI (appveyor->azure pipelines), so please hold on, I'll merge it when the move-over is done.

@atifaziz
Copy link
Contributor Author

…so please hold on…

Meanwhile, wanted to say thanks for all your prompt reviews. It's fun working on a project where the maintainer is super responsive and detail-oriented.

@atifaziz
Copy link
Contributor Author

Seems like you got the tests sorted out with 6430ed6 so I merged with master and the tests are passing (including on AppVeyor)! 🎉

@amis92 amis92 added this to the 0.5 milestone Jul 25, 2019
@amis92
Copy link
Owner

amis92 commented Jul 25, 2019

Follow-up for #28 (analyzer asserting property names are case-insensitive different, e.g. no properties like Count and count exist for a given record type - that'd mess up the methods like Update, or constructor) in a separate PR.

@amis92 amis92 merged commit cbe7b1e into amis92:master Jul 25, 2019
@atifaziz atifaziz deleted the camel-case-params branch July 25, 2019 22:49
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

2 participants