-
-
Notifications
You must be signed in to change notification settings - Fork 270
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
Add Rails/DurationArithmetic
cop
#571
Conversation
# fair | ||
Date.yesterday + 3.days | ||
created_at - 1.minute | ||
3.days - 1.hour |
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.
1.hour.until(3.days)
results in an error.
Time.current + 2.days | ||
|
||
# fair | ||
Date.yesterday + 3.days |
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 personally prefer 3.days.since(Date.yesterday)
, but sometimes it's more than just two, and:
- it gets unweildy
1.minute.until(3.days.since(Date.yesterday))
vsDate.yesterday + 3.days - 1.munute
- pattern's complexity would skyrocket
9e5cb7d
to
252e970
Compare
Checked autocorrection on
|
1f49837
to
bd409f6
Compare
# Time.current - 1.hour | ||
# | ||
# @example source that matches | ||
# ::Time.zone.now + 1.hour |
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.
Why the ::
for this example?
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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.
Ah, my bad, I thought it's in the cop's examples.
For node pattern matcher, this doc shows that both Time
and ::Time
are matched.
Backed by a proposed guideline rubocop/rails-style-guide#282
Thanks for suggestions! Please have another look. |
Thanks! |
|
||
RESTRICT_ON_SEND = %i[+ -].freeze | ||
|
||
DURATIONS = Set[:second, :seconds, :minute, :minutes, :hour, :hours, |
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'm curious if there was a specific reason that month
, months
, year
, and years
were excluded from these duration methods.
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.
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.
Backed by a
proposed✔️ guideline https://rails.rubystyle.guide/#duration-arithmeticrubocop/rails-style-guide#282, rubocop/rails-style-guide#293
TODO:
???
with PR number in changelog filerails-style-guide
StyleGuide
with a proper link to the styleguideBefore submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.and description in grammatically correct, complete sentences.
bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.