-
Notifications
You must be signed in to change notification settings - Fork 7
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
Improve code quality of extension #23
Improve code quality of extension #23
Conversation
Thanks for doing this. Struggling to run it easily until #21 is merged - shall I do that now? Building it & then installing the 'old' way with drag & drop, it's throwing an exception for me when I interact with the spinner in v0.6.0-rc3, and the dialog produced by the Java extension has weird resizing behavior. I have mixed feelings on the use of resources. While we think of it as best practice now, I think it raises the barrier of entry considerably - and brings zero benefit for anyone at the minute, since nothing else in QuPath supports switching languages. |
I see November Me also had mixed feelings on the resources. Maybe a minimal extension example, and a best practices one? Or else just include in the javadocs that resources are optional? |
I did get it to run by using some intellij magic, but it didn't seem to actually modify anything on disk. Sure, they're independent.
I thought about 2 extensions but that seems like 2x the problems. Specifying in the javadoc makes sense.
Can you be more specific? |
Rather than implement good behavior, I simply removed the bad behaviour. |
I'm still seeing
any time I attempt to change the spinner (this time running from IntelliJ, after merging the latest commits from |
Yeah sorry about that |
I'd argue that they're not super obscure in the context of a qupath extension, and while having controls that are useful might be more fun I don't know if that's necessary to demonstrate the components of making an extension |
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.
I'm in favor of making a more fun and functional extension at some stage, since I think it's a training opportunity as well as a chance to be more motivating.
But I'm also in favor of merging this as it is, as I think it's a big improvement - so from my side it can be merged.
Great. The motivation behind the integer spinner was to demo having a persistent preference that the user creates a UI for (rather than relying on propertysheet) so it'd be good to retain that. |
Resolve #22