Skip to content

CheckStyle-Rule-Extension: 6, 7, 8#4339

Merged
mssfang merged 48 commits into
Azure:masterfrom
mssfang:CS-Rule-Builder-Interface
Jul 23, 2019
Merged

CheckStyle-Rule-Extension: 6, 7, 8#4339
mssfang merged 48 commits into
Azure:masterfrom
mssfang:CS-Rule-Builder-Interface

Conversation

@mssfang

@mssfang mssfang commented Jul 9, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Check 6: @ServiceClientBuilder
    • All service client builder should be named ClientBuilder and annotated with @ServiceClientBuilder.
    • Has a method 'buildClient()' to build a synchronous client,
    • Has a method 'buildAsyncClient()' to build an asynchronous client
  • Check 7: @ServiceInterface
    • The annotation property 'name' should be non-empty
    • The length of value of property 'name' should be less than 20 characters and without space
  • Check 8: No OpenCensus dependency
    • OpenCensus should only be depended on from within the tracing-opencensus module. It should not be used in any other class throughout the Java SDK.

shafang added 15 commits July 2, 2019 11:35
Service Client Instantiation Check rule
67fb33d
add Extenal Dependentcy Exposed checks and checkstyle xml
e9db7bb
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
8dd060b 
…into CS-Rule-ServiceClient-Instantiation
revert back pom file
d8633dc
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
c428780 
…into CS-Rule-ServiceClient-Instantiation
refactor 1
ab7e6a2
Use CheckUtil to get modifier values
5c9652e
remove an extra useless boolean variable and remove unsued import
74fc583
refactor 2
a03a4a4
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
49ebc74 
…into CS-Rule-ServiceClient-Instantiation
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
3519b5e 
…into CS-Rule-ServiceClient-Instantiation
updates based on J's feedback
6daf85c
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
cf185d5 
…into CS-Rule-ServiceClient-Instantiation
revise of wording
d4b2e37
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-java …
36f5a17 
…into CS-Rule-ServiceClient-Instantiation
@mssfang mssfang self-assigned this Jul 9, 2019
@mssfang

mssfang commented Jul 12, 2019

Copy link
Copy Markdown
Contributor Author

No 9 is still experiencing the IO exception failed which is the same problem about reflection. But the existing rule implementation is ready for review.

JonathanGiles
JonathanGiles requested changes Jul 12, 2019
View reviewed changes
Comment thread core/azure-core/src/main/java/com/azure/core/implementation/RestProxy.java Outdated
Comment thread core/azure-core/src/main/java/com/azure/core/implementation/RestProxy.java Outdated
JonathanGiles
JonathanGiles reviewed Jul 12, 2019
View reviewed changes
Comment thread ...ality-reports/src/main/java/com/azure/tools/checkstyle/checks/ServiceClientBuilderCheck.java Outdated
Comment thread ...quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ThrownClientLoggerCheck.java Outdated
@conniey

conniey commented Jul 12, 2019

Copy link
Copy Markdown
Member

I think these checks might have to be split up into another PR. (ie. Check 9 about logAndThrow will result in a PR that touches many files.)

@mssfang mssfang force-pushed the CS-Rule-Builder-Interface branch from d6d9dee to aba32d2 Compare July 12, 2019 06:27
@mssfang mssfang changed the title CheckStyle-Rule-Extension: 6, 7, 8, 9 CheckStyle-Rule-Extension: 6, 7, 8 Jul 12, 2019
@mssfang mssfang requested a review from g2vinay July 12, 2019 17:42
@mssfang mssfang requested a review from JonathanGiles July 12, 2019 21:34
@mssfang

mssfang commented Jul 12, 2019

Copy link
Copy Markdown
Contributor Author

Cleaned up and ready for review again. :>

srnagar
srnagar requested changes Jul 15, 2019
View reviewed changes
Comment thread ...ality-reports/src/main/java/com/azure/tools/checkstyle/checks/ServiceClientBuilderCheck.java Outdated
Comment thread ...e-quality-reports/src/main/java/com/azure/tools/checkstyle/checks/ServiceInterfaceCheck.java Outdated
Comment thread ...ality-reports/src/main/java/com/azure/tools/checkstyle/checks/ServiceClientBuilderCheck.java Outdated
@mssfang mssfang removed the request for review from g2vinay July 18, 2019 17:02
@mssfang mssfang requested a review from srnagar July 18, 2019 20:29
@mssfang

mssfang commented Jul 22, 2019

Copy link
Copy Markdown
Contributor Author

PR is ready for reviewing again

JonathanGiles
JonathanGiles approved these changes Jul 23, 2019
View reviewed changes

@JonathanGiles JonathanGiles left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved with one change requested first.

Comment thread .../azure-keyvault-secrets/src/main/java/com/azure/security/keyvault/secrets/SecretService.java Outdated
srnagar
srnagar approved these changes Jul 23, 2019
View reviewed changes

@srnagar srnagar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@mssfang mssfang merged commit 7b18d30 into Azure:master Jul 23, 2019
@mssfang mssfang deleted the CS-Rule-Builder-Interface branch July 23, 2019 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JonathanGiles JonathanGiles JonathanGiles approved these changes
@srnagar srnagar srnagar approved these changes
@conniey conniey Awaiting requested review from conniey
@danieljurek danieljurek Awaiting requested review from danieljurek
@mitchdenny mitchdenny Awaiting requested review from mitchdenny
@weshaggard weshaggard Awaiting requested review from weshaggard

Assignees

@mssfang mssfang

Labels

Java Source Code Rules

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mssfang @conniey @JonathanGiles @srnagar