Skip to content

Fix several inspections #526

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

Closed
9 tasks done
FWDekker opened this issue Jan 15, 2024 · 0 comments · Fixed by #527
Closed
9 tasks done

Fix several inspections #526

FWDekker opened this issue Jan 15, 2024 · 0 comments · Fixed by #527
Assignees
Labels
code quality Code changes without behavior changes

Comments

@FWDekker
Copy link
Owner

FWDekker commented Jan 15, 2024

I ran some inspections on the code base and found the following potential issues:

  • InsertAction uses a form of runWriteCommandAction which is annotated as @TestOnly. I should use the one with a different signature, where I also specify the action's name.
  • radioButtonGroupProp in AssertJSwingProperties is never used. Remove it.
  • DefaultTemplateActionLoader should (apparently) not be public (but internal).
  • Various fields in plugin.xml should support i18n. (Re-run the inspection to see which ones I mean.)
  • StringScheme.Companion#DEFAULT_CAPITALIZATION should be = instead of get() =.
  • Do I still need the com.vdurmont:emoji-java dependency? (Adds tests for this!) (Edit: Emoji are not explicitly supported, but also don't need explicit support like they used to.)
  • If I'm looking at dependencies anyway, document once and for all how it works with the kotlin-reflect dependency.
  • Consider adding explicit return types for most methods (enable this as an inspection where possible). (In Detekt?) Don't fix this right away everywhere, that'd be nasty. Make it gradual. Nope, looks ugly in many cases. I'll decide on a case-by-case basis.
  • Use ProjectActivity instead of StartupActivity to notify users. (Test this in various versions!) (Edit: Tested. Both implementations work in both 2023.1 and 2023.3. I'll stick with StartupActivity because that's the more modern one.)

Also, I can

  • add a message to the automatic error reports indicating that users can subscribe to the issue to get updates, and add information on how they can delete the message, and ensure the privacy notice in the error reporter itself mentions GitHub.
  • Also, change the GitHubScrambler to use actual scrambling instead of overly fancy encryption. (Edit: Not that important, really.)
@FWDekker FWDekker added the code quality Code changes without behavior changes label Jan 15, 2024
@FWDekker FWDekker self-assigned this Jan 15, 2024
@FWDekker FWDekker moved this to Ready in Randomness issues Jan 15, 2024
@FWDekker FWDekker moved this from Ready to In progress in Randomness issues Jan 16, 2024
FWDekker added a commit that referenced this issue Jan 16, 2024
FWDekker added a commit that referenced this issue Jan 16, 2024
@FWDekker FWDekker moved this from In progress to In review in Randomness issues Jan 17, 2024
@github-project-automation github-project-automation bot moved this from In review to Done in Randomness issues Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Code changes without behavior changes
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant