Skip to content

Rake notes compatibility in Rails 5.1.0 #1661

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

Merged
merged 4 commits into from
Jul 9, 2016

Conversation

johnmeehan
Copy link
Contributor

As of Rails 5.1.0 gems can register a directories to work with rake notes

in this case we can tell Rails it to now look in the /spec folder when searching for annotations like # TODO: # FIXME etc.

This was added in this PR:
rails/rails#25692

Resulting in:

$ rake notes
app/spec/models/article_spec.rb:
  * [2] [TODO] add more specs
  * [3] [FIXME] Spec A is broken
  * [4] [OPTIMIZE] improve this test

Thats one less daily annoyance.

John

Rails SourceAnnotationExtractor which means when we run `rake notes` it will now also
for the first time list out any todos in the `/spec` folder.

This was added in this PR:
rails/rails#25692

Resulting in:

```bash

$ rake notes
app/spec/models/article_spec.rb:
  * [2] [TODO] add more specs
  * [3] [FIXME] Spec A is broken
  * [4] [OPTIMIZE] improve this test

```
* just fixed the railtie

As of Rails 5.1.0 gems can register a directories to work with `rake
notes`

In this case we can tell Rails it to now look in the `/spec` folder when
searching for annotations like `# TODO:` `# FIXME` etc.

This was added in this PR:
rails/rails#25692

Resulting in:

```bash

$ rake notes
app/spec/models/article_spec.rb:
  * [2] [TODO] add more specs
  * [3] [FIXME] Spec A is broken
  * [4] [OPTIMIZE] improve this test

```

Thats one less daily annoyance.

John
@@ -8,6 +8,12 @@ module RSpec
module Rails
# Railtie to hook into Rails.
class Railtie < ::Rails::Railtie

# As of Rails 5.1.0 you can register directories to work with `rake notes`
if Gem::Requirement.new(">= 5.1.0").satisfied_by?(Gem::Version.new(::Rails.version))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you switch this away from using rubygems please? It's generally a good idea to not assume it's present, a check on the rails version string is sufficient normally (e.g. ::Rails.version >= "5.1.0").

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can see our preferred form a few lines down in fact, so making this ::Rails::VERSION::STRING >= '5.1' would be preferred :)

* Changed the method of detecting the Rails version.
* Removed extra spaces.
It works with 5.1 and greater. Not just over 5.1 :)

Rails::VERSION::STRING >= '5.1'
@JonRowe JonRowe merged commit 73b742c into rspec:master Jul 9, 2016
@JonRowe
Copy link
Member

JonRowe commented Jul 9, 2016

Thanks ❤️

JonRowe added a commit that referenced this pull request Jul 9, 2016
sebjacobs pushed a commit to futurelearn/rspec-rails that referenced this pull request Mar 15, 2019
* As of Rails 5.1.0 gems can register a directories with the
Rails `SourceAnnotationExtractor` which means when we 
run `rake notes` it will now also list out any todos in the
`/spec` folder.

This was added in this PR:
rails/rails#25692

Resulting in:

```bash

$ rake notes
app/spec/models/article_spec.rb:
  * [2] [TODO] add more specs
  * [3] [FIXME] Spec A is broken
  * [4] [OPTIMIZE] improve this test

```

* In this commit:
* just fixed the railtie

As of Rails 5.1.0 gems can register a directories to work with `rake
notes`

In this case we can tell Rails it to now look in the `/spec` folder when
searching for annotations like `# TODO:` `# FIXME` etc.

This was added in this PR:
rails/rails#25692

Resulting in:

```bash

$ rake notes
app/spec/models/article_spec.rb:
  * [2] [TODO] add more specs
  * [3] [FIXME] Spec A is broken
  * [4] [OPTIMIZE] improve this test

```
sebjacobs pushed a commit to futurelearn/rspec-rails that referenced this pull request Mar 15, 2019
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.

2 participants