-
-
Notifications
You must be signed in to change notification settings - Fork 266
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
Copyedit Rails/RakeEnvironment cop documentation. #164
Conversation
@@ -334,7 +334,7 @@ Rails/Present: | |||
UnlessBlank: true | |||
|
|||
Rails/RakeEnvironment: | |||
Description: 'Set `:environment` task as a dependency to all rake task.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also capitalize Rake here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thank you.
# models, without `environment` dependency. | ||
# This cop checks for Rake tasks without the `environment` dependency. The | ||
# `environment` task loads application code for other Rake tasks. Without | ||
# it, tasks cannot make use application code like models. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing "of" after "use"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
1eec660
to
4eb039f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!! 👍
# `environment` dependency is important because it loads application code | ||
# for the rake task. The rake task cannot use application code, such as | ||
# models, without `environment` dependency. | ||
# This cop checks for Rake tasks without the `environment` dependency. The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the document will be more consistent with :environment
task.
-This cop checks for Rake tasks without the `environment` dependency. The
+This cop checks for Rake tasks without the `:environment` task dependency. The
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea. I made the change
# for the rake task. The rake task cannot use application code, such as | ||
# models, without `environment` dependency. | ||
# This cop checks for Rake tasks without the `environment` dependency. The | ||
# `environment` task loads application code for other Rake tasks. Without |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-# `environment` task loads application code for other Rake tasks. Without
+# `:environment` task loads application code for other Rake tasks. Without
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
# | ||
# You can ignore the offense if the task satisfies at least one of the | ||
# following conditions: | ||
# | ||
# * The task does not need application code. | ||
# * The task invokes :environment task. | ||
# * The task invokes the :environment task. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-# * The task invokes the :environment task.
+# * The task invokes the `:environment` task.
4eb039f
to
95b1746
Compare
* Tweak wording of offense message and supporting documentation to increase readability.
95b1746
to
e43c772
Compare
Should I add a note about this in the changelog? |
No changelog entry is required for this change. Thank you for the improvement! |
The new
Rails/RakeEnvironment
task from @pocke is great! 😄 But there are a few copy edits that could make the offense more readable.This PR tweaks the wording of the offense and supporting documentation to increase readability. This is somewhat subjective, so let me know what you think!
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).AddedUpdated existing tests.and description in grammatically correct, complete sentences.
bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.