Performance cops have to extracted into a standalone gem (named something like rubocop-performance or whatever).
The reason for this decision is simple. I want RuboCop to be focused just on Ruby and have no framework/library/runtime dependant pieces for the sake of being easier to focus on evolving it. Creating more extensions should also help us come up with a good official API for them. Performance cops are all MRI focused and are highly dependent of the version of MRI you're using, so they don't belong in the core RuboCop.
Any volunteers to do this? I was always ambivalent towards the performance checks (as they often come at the expense of readability), so I'm not particularly interested in working on this.
Any volunteers to do this? I was always ambivalent towards the performance checks (as they often come at the expense of readability), so I'm not particularly interested in working on this.
Agreed. Language level optimizations also suffer from being subject to the current VM implementation. So a big task for whoever wants to take this would be to benchmark on every Ruby version and see that the improvements still hold.
Although they just extract it as-is in the beginning and improve things later. Probably performance cops should have explicit versions on which they make sense as part of their config.
Extracting as it is into a gem seems simple enough. I could handle this one if nobody is on it yet.
I also like the idea of showing instructions on how to add the gem if they have performance cops in their configs, the same way you suggested on #5976.
@gizotti Happy to hear this!
I've created a placeholder repo for the extraction https://github.com/rubocop-hq/rubocop-performance
@bbatsov can you just add a README on the repo please? I can't fork an empty repo :)
Done.
Oh darn, I didn't see that you wanted to take this on @gizotti. I opened https://github.com/rubocop-hq/rubocop-performance/pull/1 but maybe we can work together on it.
If you are already further along than I was, I can also just close the PR. I should have checked the issue again before doing anything.
No worries @composerinteralia, I got to where you were last night so not reason to push my stuff if you already did.
I'll leave you to it, I think if we both work in the same branch we might step on each others toes.
I'm investigating the same git history problem as the current RuboCop Rails repository. The current situation is below.
https://github.com/rubocop-hq/rubocop/issues/5976#issuecomment-425884216
@bbatsov @composerinteralia
Cc @rubocop-hq/rubocop-core
I extracted lib/rubocop/cop/performance/* and spec/rubocop/cop/performance/* along with git history from RuboCop Core (I used git cherry-pick for this extraction) .
I think that lib/rubocop/cop/performance and spec/rubocop/cop/performance will be the most important history in maintenance.
The following branch including previous work done in RuboCop Performance repo, and it is synchronized with the latest performance cops of RuboCop Core.
https://github.com/koic/rubocop-performance/tree/import_performance_cops_with_git_history
If it's OK, force-push this branch to master of RuboCop Performance repo.
https://github.com/rubocop-hq/rubocop-performance
After that, we can restart work based on this git history for this issue.
馃摑 @amatsuda told me the experiecce that how to divided Kaminari repo at Asakusa.rb. So I was able to obtain great knowledge. Thank you!
@koic, @amatsuda 馃檱
If it's OK, force-push this branch to master of RuboCop Performance repo.
Yeah, it's OK. Make this happen!
P.S. Great work, probably should document the process somewhere for future reference.
Thanks! I force-pushed it.
This was a very steady work 馃槄, but I'm thinking of share know-how somewhere.
@bbatsov Thank you for waiting. We can release the rubocop-performance gem.
We need to decide on the rubocop-performance version to be released. The version number should be a number greater than 0.61.
https://github.com/rubocop-hq/rubocop-performance/pull/26#pullrequestreview-213288088
Also, I'm ready to remove performance cops from rubocop-hq/rubocop repo, but before that we need to notify the users in some way.
Anyway, it is good to release the rubocop-performance gem.
Fantastic work!
Let's go with version 1.0 for rubocop-performance.
Also, I'm ready to remove performance cops from rubocop-hq/rubocop repo, but before that we need to notify the users in some way.
Let's just add some message that say on gem install that the cops are going to be moved to a different gem. Can't think of any better way to notify them about the change, so I hope this will be enough.
Thank you for the advice!
First I will release rubocop-performance 1.0. After that, I will open a PR to RuboCop core which will fallback to rubocop-performance with the message.
馃憤
rubocop-performance 1.0.0 has been released 馃殌
https://rubygems.org/gems/rubocop-performance/versions/1.0.0
Fantastic!
I'll make an announcement once we've made the necessary changes to RuboCop itself.
I opened #6871 proposing to rescue some Performance cops and upgrading them to Style namespace.
I've just realised that you used cherry-pick to keep the history in the extracted extension.
If you happen to face a similar task in the future, I highly recommend using git-filter-repo (but be careful, it's destructive to an extent where reflog doesn't help).
馃憤 That's what I used to extract rubocop-ast
@pirj Looks pretty cool! Thanks for sharing it!
@pirj 馃憤 I can't remember what was wrong, I remember giving up on git filter-branch because something couldn't be achieved at that time. And I didn't know "git-filter-repo", it could have been easier with it! 馃槄 Great thank you for letting me know.
Most helpful comment
Thanks! I force-pushed it.
This was a very steady work 馃槄, but I'm thinking of share know-how somewhere.