It’s common that you want to run Rubocop on your plugin. In various plugins I’ve seen the pattern that it defines some rubocop task. However, most rake tasks are executed from the Foreman checkout itself. Standardizing this also makes it easier to run in CI. This is why I opened a PR to run Rubocop in a plugin.
The workflow is:
git clone foreman.git and add my_plugin as a dependency (as you normally would)
Run bundle exec rake plugin:rubocop[my_plugin] from the foreman checkout
It will use .rubocop.yml from the plugin if present. You can also pass another argument to output a JUnit XML file which can be very useful in CI.
Let me know if I missed anything. If it’s merged, I’m proposing to cherry pick it to stable branches so we can update our Jenkins setup to use it.
I don’t believe we should do this, unless we standardize the default RoboCop config.
I’m running RoboCop just by running rubocop cli command locally in the plugin directory and I think we shouldn’t imply the foreman dependency unless we need to do so.
So if we manage to get RoboCop defaults from Foreman, I’m all in, otherwise I’d standardize through the CI directly.
I see a strong trend where plugin maintainers prefer follow their own styles.
From a community perspective, that does mean that you have to be much more aware of which project you’re working on.
That leads me to think there are generally 3 options, which I’ll explain:
The shared nothing option means everything (rubocop version, rubocop config, maybe even another tool) lives in the plugin. The Jenkins job will simply chdir to the to the plugin, bundle install and run bundle rake $job
The next is the same, but if some marker is not present (not sure what this should be, maybe the presence of a file in the plugin git repo), it falls back to some shared config that lives in Foreman’s repo.
The last is basically the fallback option from the previous situation.
Shared nothing, all in the plugin
Default to plugin, fall back to shared
All from Foreman
The trade off is control of the plugin author. This freedom does mean a plugin author needs to do the work of maintaining. On the other hand, it does also mean that if the core project decides something, plugin authors do not need to do anything. The full copy may also work better with tools like Hound.
I’d like to ask plugin authors what their preference is.
I’d love this. As I’m very supportive of moving what we can to GH actions, I’d like to keep it simple and cloning foreman just for RoboCop is not that great, but our own gem with just the config sounds great. Btw, it could be just a GH action, with shared config.
Let me make some prototype over the weekend. But gem has more general usability anyway.
I do not contribute the code that much anymore and can live with whatever obscure cops.
However as a plugin maintainer, I’d prefer toto enforce my style or better to say, disable cops I don’t like. Every rubocop update surprises me with some rule, that was never needed before. If an update in core, out of a sudden, breaks my plugin tests on every PR, it will cause a frustration on my end. Especially if it happens just before the release.
Looking at it from the contributor perspective, our rubocop definition is highly customized. I don’t think someone would first look at rubocop.yml and code according to it. They just write the code, push the PR, see what they need to fix, run rubocop to fix it and repush. Therefore I don’t think it’s a big deal if plugins have different definitions.
But if more people prefer to unify this, I’m not against it. I just feel it’s not worth the effort.
So the real reason the bootdisk job is failing is that every plugin adds Rubocop to jenkins:unit. That means it’s running the Rubocop job from the other gem in bootdisk.
To solve it, we still need to find a new way in Jenkins jobs. There are only 2 votes, but they both were for Shared nothing, all in the plugin. That suggests we should do bundle install in the plugin directory and run some job. This could be rake rubocop, rake jenkins:rubocop or plain rubocop. We should ensure they write the XML file to a predictable location so Jenkins can parse the results (something we don’t do today). This can be trivially done via plain rubocop, but that makes it harder to configure the job via git. A jenkins:rubocop can always write the file, but requires additional work on each plugin. We can also suggest plugin authors to write the task like this (untested):
RuboCop::RakeTask.new(:rubocop) do |task|
task.formatters = ['autogenconf', 'junit']
task.options = ['--out', ENV['RUBOCOP_JUNIT']]
Jenkins would set the env var RUBOCOP_JUNIT.
Once that’s done, all plugins should stop running Rubocop as part of jenkins:unit to avoid duplicate work.
IMHO this is not blocked on any work to make configuration easier. Today most plugins already have some Rubocop config.
I would say we should drop this from jenkins:unit. Most plugins anyways run rubocop using travis/hound/github actions already, and it doesn’t really make sense to have it run with a different config on jenkins. If we want to run rubocop on jenkins we could do it as a separate task from the tests, but I don’t see much value in doing that.
That’s where I was heading as well. The rubocop is perfect candidate to get extracted to github action, as the response is imediate, so it serves instead of annoying hound (notify me in matter of minutes, that i need to reformat and thus I am more likely to do it right away).
That’s why I’m inclined for easy solution. Shared config in foreman core, that offers basic cops, that we think all the plugins should use is what my PR would offer.
The url referenced cop is actually downloaded locally and checked only once in a while (can be include in git), probably not hard failing if no internet connection available.
I agree that versioning of it can be an obstacle, but on the contrary it doesn’t change as often.
Plus it’s not as hard to move to a gem, once we see that the versioning is an real issue.
Just every change to common cops and I don’t see that happening very often. Gem should be released automatically, you just need to bump the gem version in you gemfile if you wanna get the updated core cops. So I don’t need to follow the core .rubocop file if I want to have the same style as core.
You are still kept with that option, if plugin maintainer don’t want to follow core guidelines, noone forces him to. It would only provide an option for standardized RuboCop rules over the ecosystem.
We should promote this, what we have today is so confusing. Here is a scenario I am seeing every month: a random contributor fixes something in a plugin, small patch, tests pass but Rubocop does not. She tries to run Rubocop in the plugin repo - it either does not work (Rubocop is not present in Gemfile) or what’s worse it runs some different version of Rubocop with a different configuration. This stems from the fact that even us, engineers, don’t know how Rubocop is executed on our CI.
This is super confusing and what I am fighting for here is that we get rid of this completely. I hate when we do something that people do not expect. Like using non Ruby on Rails technologies in RoR application. Or using Rubocop in such way that nobody would ever figure that out. We are wasting our and others time.
Don’t get me wrong, pal. I get your idea. I just think this isn’t getting any better.