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

Fix a few method redefinition warnings #1459

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

Earlopain
Copy link
Collaborator

2 enum values conflict with already existing scopes: "retried" and "discarded" (oops) (#1420)

"running" and "finished" got moved from the discrete execution cleanup (#1427). This change I'm not so certain about which of these two is the correct one. Tests do pass but you'd better check these two anyways, this just removes the earlier definition:

# first
scope :running, -> { where.not(performed_at: nil).where(finished_at: nil).joins_advisory_locks.where.not(pg_locks: { locktype: nil }) }
# second
scope :running, -> { where.not(performed_at: nil).where(finished_at: nil) }

# first
scope :finished, -> { where.not(finished_at: nil) }
# second
scope :finished, ->(timestamp = nil) { timestamp ? where(arel_table['finished_at'].lteq(bind_value('finished_at', timestamp, ActiveRecord::Type::DateTime))) : where.not(finished_at: nil) }

Earlopain and others added 2 commits August 6, 2024 21:04
2 enum values conflict with already existing scopes: "retried" and "discarded" (bensheldon#1420)
"running" and "finished" got moved from the discrete execution (bensheldon#1427)
@bensheldon
Copy link
Owner

Thank you so much for catching these! I must have something wonky in my test setup that I'm not seeing these warnings.

I just chose the other versions 😁 There is already a finished_before method that did the same thing as taking an argument.

@bensheldon bensheldon added the refactor Code changes that do not introduce new features label Aug 8, 2024
@bensheldon bensheldon merged commit 72ba713 into bensheldon:main Aug 8, 2024
10 checks passed
@Earlopain
Copy link
Collaborator Author

Looks good, thanks. You just have to set $VERBOSE = true for these to appear (which I tend to do). I would have done this here already but there are two other minor things to get out of the way first.

@Earlopain Earlopain deleted the scope-redefinition-warning branch August 8, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code changes that do not introduce new features
Projects
Development

Successfully merging this pull request may close these issues.

2 participants