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

Rspec cop #1087

Closed
wants to merge 2 commits into from
Closed

Rspec cop #1087

wants to merge 2 commits into from

Conversation

geniou
Copy link
Contributor

@geniou geniou commented May 13, 2014

I like the idea / convention not to use should at the beginning of a test description - as proposed here: http://betterspecs.org/#should

This cop is looking for should at the beginning of a test description. Maybe in the future this cop could do more rspec related tests.

@bbatsov
Copy link
Collaborator

bbatsov commented May 13, 2014

This has already been proposed and rejected in the past. See #731. In hindsight, I regret adding even the existing Rails support and plan to move it to a separate gem at some point.

RuboCop's core should be free of framework specific cops/code.

@geniou
Copy link
Contributor Author

geniou commented May 13, 2014

I get your point, but I like this cop and I think its worth having it (as you can see in the second commit). Do you already have an idea how "plugins" like rails or rspec can be integrated in the future? I've seen that there is a rubocop-rspec - is this the way to go?

@bbatsov
Copy link
Collaborator

bbatsov commented May 13, 2014

That's the best way at the moment, yes. Not sure we need something much more complex than this.

I'm not saying the cop is bad - it's certainly useful. But this doesn't change the fact it does not belong in RuboCop. Maybe you can work with @nevir on improving rubocop-rspec? I think it'd be great if there were cops covering most of the rules in betterspecs.

@geniou
Copy link
Contributor Author

geniou commented May 13, 2014

Ok - than I have two questions:

  1. Would you then be willing to add this extension to the rubcop tests, since rubocop is using rspec?
  2. Is there a way to specify the required files in the rubocop config, so that I can run rubocop without the --require option, and it will automatically also load the extension file?

@jdickey
Copy link

jdickey commented May 14, 2014

RSpec should is deprecated and people are encouraged to write new specs using the expect syntax. (Zero monkey patching and so on; read the linked maintainer's blog for more). It's harder to make the mental switch than the code switch but, three months on, I seem to write cleaner specs. YMMV, of course, but adding infrastructure for RSpec should at this point ranks right up there with a buggy-whip holder for your new Corvette; rather beside the point.

@geniou
Copy link
Contributor Author

geniou commented May 14, 2014

@bbatsov What do you think about creating an organization here in gihub, that can host the rubocop extensions (like rspec, or in the future rails). With this its bundled and has nice urls like github.com/rubocop/rubocop-rspec or github.com/rubocop/rubocop-rails instead of github.com/user1/rubocop-rspec and github.com/user2/rubocop-rails. Its just an idea - let me know what you think.

@bbatsov
Copy link
Collaborator

bbatsov commented May 14, 2014

I'm not opposed to the idea. There are other RuboCop related projects that might live there as well I guess (guard-rubocop and rubocop-emacs) come to mind. Anyways, let's first hear that @jonas054 and @yujinakayama think about a RuboCop org.

@jonas054
Copy link
Collaborator

I'm not sure if I grasp all implications but it sounds like a good idea to me.

@philoserf
Copy link

+1 for ruby core cops being separate from rails cops

@yujinakayama
Copy link
Collaborator

I'm on the fence about the RuboCop organization. Maybe we can introduce it when we will extract the Rails cops, but I don't see more advantages than disadvantages (requires users to update the URL in the repo or Gemfile) right now.

@nevir
Copy link
Contributor

nevir commented May 15, 2014

@yujinakayama repo transfers redirect now - it should be transparent to clients

+1 for the rubocop org for me; I'd gladly donate rubocop-rspec to the cause :P

@bbatsov
Copy link
Collaborator

bbatsov commented May 18, 2014

Yeah, the URL should not be a problem. I've had a pretty good experience with migrating projects to clojure-emacs organization recently. That said, I'd like us to focus on that particular ticket, since I don't like having too much open issues. @geniou Please, update the PR to feature just the second commit.

@geniou
Copy link
Contributor Author

geniou commented May 18, 2014

@bbatsov since there are now several new cops in rubcop-rspec there are other things to be cleared as well - I will provide an separate PR for this.

@bbatsov
Copy link
Collaborator

bbatsov commented May 18, 2014

@geniou Great!

@geniou geniou mentioned this pull request May 18, 2014
@geniou geniou closed this May 18, 2014
@geniou
Copy link
Contributor Author

geniou commented May 18, 2014

I closed a this PR and opened a new one for the cleanup, and will open one as soon as rubocop-rspec is ready to be integrated.

@geniou geniou deleted the rspec_cop branch May 18, 2014 08:09
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

Successfully merging this pull request may close these issues.

7 participants