-
Notifications
You must be signed in to change notification settings - Fork 653
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
[IJ Plugin] Add 'Input class constructor issue' inspection #5427
Conversation
✅ Deploy Preview for apollo-android-docs canceled.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bunch of wording nitpicks but looks great overall!
intellij-plugin/src/main/resources/inspectionDescriptions/ApolloInputConstructorNamedArgs.html
Outdated
Show resolved
Hide resolved
intellij-plugin/src/main/resources/inspectionDescriptions/ApolloInputConstructorNamedArgs.html
Outdated
Show resolved
Hide resolved
intellij-plugin/src/main/resources/inspectionDescriptions/ApolloInputConstructorNamedArgs.html
Outdated
Show resolved
Hide resolved
intellij-plugin/src/main/resources/inspectionDescriptions/ApolloInputConstructorNamedArgs.html
Outdated
Show resolved
Hide resolved
implementationClass="com.apollographql.ijplugin.inspection.ApolloInputConstructorNamedArgsInspection" | ||
groupPathKey="inspection.group.graphql" | ||
groupKey="inspection.group.graphql.apolloKotlin" | ||
key="inspection.inputConstructorNamedArgs.displayName" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can there be a description that explains the issue in more depth?
intellij-plugin/src/main/resources/messages/ApolloBundle.properties
Outdated
Show resolved
Hide resolved
Co-authored-by: Martin Bonnin <martin@mbonnin.net>
context(LocalInspectionTool) | ||
fun ProblemsHolder.registerProblem( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤯 Do we have context receivers in the codebase now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahaha yeah actually I've used them before in the plugin code 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plugin is probably OK but I'm not sure about the compatibility implication of those so I'd rather not use them in the libs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed 👍 We have a bit more leeway in the plugin.
Screen.Recording.2023-12-04.at.19.15.58.mov
Fix for #5374