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

Always use latest Checkstyle version #373

Open
erikhuizinga opened this issue Jan 24, 2018 · 13 comments
Open

Always use latest Checkstyle version #373

erikhuizinga opened this issue Jan 24, 2018 · 13 comments

Comments

@erikhuizinga
Copy link

erikhuizinga commented Jan 24, 2018

As a different approach to #199, I'd like to always use the latest major or latest minor version of Checkstyle. In IDEA's settings, I can choose from a large dropdown of Checkstyle versions.

As a user, I just want the latest Checkstyle version to be enabled. Currently, after the plugin updates, I have to manually go to the plugin settings and choose the latest version (which often, but not always has a newer version). This is a somewhat unnecessary hassle, because I think it would be possible to have an option 'latest version' and/or 'latest minor version'. In this way I would not have to worry about plugin updates anymore. The hassle is even bigger for me, because I have the plugin installed in both IDEA and Android Studio, which means double the hassle after updates.

For a better UX, upon plugin update, the plugin could notify the user that it has upgraded the Checkstyle version as well, so the user still knows about it.

@tsjensen
Copy link
Contributor

tsjensen commented Jan 24, 2018

I understand. Always activating the latest version would be possible as long as you use the bundled configurations only.
Once you start using your own configuration file, things will certainly start to break, because the Checkstyle team makes breaking changes on most releases, even on minor or patch level releases. This is why normally you never want to change versions automagically. Also consider the build process, which needs to know about the version and should use the same version as your IDE. Or centralized inspection services such as SonarQube ...
All in all, I am somewhat unsure what to do about this. We might have an option that gets disabled once you add a custom config file? But then, what about the build process etc?

@jshiell
Copy link
Owner

jshiell commented Jan 25, 2018

I can understand the desire for this, but I'm very wary of adding support for such. One of the primary reasons is that when the plugin was always tied to the latest version there was a constant stream of support requests from people who'd upgrade and find their config files no longer worked with the latest Checkstyle. Especially since they seem to have been making a lot more non-backwards-compatible changes since ~8ish.

I do think the update notification is a great idea though - as unless you read the change-log there's no current way of communicating the new availability.

@erikhuizinga
Copy link
Author

Doesn't the plugin come with the Sun and Google checks bundled? Have they always been compatible with the latest Checkstyle spec? An option 'latest compatible' for either of the two could be an option, or something similar.

Thanks for the serious consideration. I just hope to help your users have a smooth update experience. Currently it simply isn't when I find out Checkstyle v8.7 is out while I'm still on v8.3. I feel it's not my job as a simple user to keep updating all my plugins and their dependencies, but I should keep myself informed. A notification through the update dialog is a logical tool to achieve this.

@tsjensen
Copy link
Contributor

About your question: The Checkstyle guys ship a Sun and Google config file with every version they publish, and it's compatible with only that version. So we also include those files for every version that we support, and use the one that fits your selected version of Checkstyle.

@erikhuizinga
Copy link
Author

In that case, the latest Google or Sun checks could be automatically selected upon update if the user so chooses.

@tsjensen
Copy link
Contributor

Correct! Like I said, we might have an option that gets disabled once you add a custom config file. But I am still inclined to follow @jshiell and not do it, because of the follow-up problems this will cause (build process, continuous inspection server won't be in sync).

@jshiell
Copy link
Owner

jshiell commented Jan 27, 2018

I've added support for pointing users at the release notes when the plugin is updated. It's raw, but a start.

Tracking the matching Google & Sun checks only seems a reasonable feature. Not sure how easy it'd be, though. Were I to start the project anew the model would be built on a configuration location being tied to a version of Checkstyle (which could potentially be 'latest'), which would make this easy. But if wishes were horses ... instead, our model was built around a global, moving version of Checkstyle, and a bunch of rule files independent of that. The good @tsjensen managed to get the multiple Checkstyle version support in there, but it doesn't resolve my lack of foresight a decade ago.

So, here's the punch-line - this seems a good idea, and I can clearly see the use of it. However, it's unlikely to happen without a major redesign of the model. And that's pretty unlikely at present - I very much consider this a care & maintenance project from my perspective. I don't use it, nor would I even recommend it to teams I work on, for reasons which are a discussion in themselves. But the result is that anything other than minor changes/fixes happen only when I wake up with an insatiate need to fix some of the tangle that is this project (or something else does the same). And I fear that's pretty rare at present.

@erikhuizinga
Copy link
Author

That reasonable reasoning. ;)

If you don't feel the need to make this feature, then thank you for considering making it and even implementing an idea you got through this issue. Maybe someday somebody will come along, fork this project (at least the idea) and rebuild it so that Checkstyle's bundled checks and custom checks and dynamic Checkstyle version selection can be supported. It's FOSS after all!

Maybe keep this issue open and add the 'help wanted' tag. Some people search for it and may have time to commit themselves to such a feature.

@tsjensen
Copy link
Contributor

tsjensen commented Feb 4, 2018

Frankly, I think that one cannot use Checkstyle in earnest and auto-update it. That's just not how this tool works (even though I wish it was - it wouldn't even be very complicated).
So, I think we should close this issue as "won't fix", and possibly open a new one to remind us of any planned improvements to the notification feature.

@micke239
Copy link

micke239 commented Feb 5, 2018

8705650#diff-60294761158786a067d6b53700592efe

This commit added "last-active-plugin-version" to the checkstyle-idea.xml, which makes things complicated for us who commit to assert that all team members are using the same config. The file is now modified each time someone updates the plugin.

Was that intentional? Seems like it doesnt have anything with this issue to do at all.

@jshiell
Copy link
Owner

jshiell commented Feb 5, 2018

Yes. Related to:

I've added support for pointing users at the release notes when the plugin is updated. It's raw, but a start.

@micke239
Copy link

micke239 commented Feb 5, 2018

Is it really worth it though? It really makes it harder to distribute the plugin configuration.

@jshiell
Copy link
Owner

jshiell commented Feb 5, 2018

Well, yes, for some. For your use case this makes life harder; for others, it'll make life easier.

In theory (as I'm at work and my mind is very much elsewhere) this could be mitigated by added some workspace config, and storing this setting there. Seems a reasonable suggestion, but see comments above re my current effort devoted to the project (i.e. it might happen tomorrow, or never).

Edit: it's not too hard to move to the workspace, but will take some refactoring to decouple the configuration from a 1-1 relationship with the persistence component, so we can have two (project + workspace persistent components).

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

No branches or pull requests

4 participants