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

Batch #2 - 11. No .builder() API: No classes should have a .builder() #4589

Merged
merged 2 commits into from
Jul 26, 2019

Conversation

vhvb1989
Copy link
Member

add MethodName rule to disable builder as a method name and update isssues

You might want to see checkstyle.xml for MethodName rule update. Then everything else is fixing what we had already with a builder()

<module name="MethodName"/>
<module name="MethodName">
<!-- Use default MethodName but block the use of 'builder' as method name -->
<property name="format" value="(?=^[a-z][a-zA-Z0-9]*$)(?!^[b][uU][iI][lL][dD][eE][rR]$)"/>
Copy link
Member

@conniey conniey Jul 25, 2019

Choose a reason for hiding this comment

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

Are mode specifiers available? It's easier to read ((?i)builder)$, imho.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is supported. I have tested and worked.
Thank you for it! 👍 Updated now

Copy link
Member

@conniey conniey left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@mssfang mssfang left a comment

Choose a reason for hiding this comment

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

LGTM

@vhvb1989 vhvb1989 merged commit 9739dc6 into master Jul 26, 2019
- *
- * @return A new {@link HttpPipelineBuilder} to create a HttpPipeline from.
*/
public HttpPipelineBuilder() {
Copy link
Member

Choose a reason for hiding this comment

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

Can you please fix up the javadoc here. Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants