Skip to content

Remove some unused(?!) methods on contest_relation#315

Open
bagedevimo wants to merge 1 commit intomigration-versionfrom
remove-contest-finish-at-with
Open

Remove some unused(?!) methods on contest_relation#315
bagedevimo wants to merge 1 commit intomigration-versionfrom
remove-contest-finish-at-with

Conversation

@bagedevimo
Copy link
Contributor

@bagedevimo bagedevimo commented Jul 4, 2025

Based on PR #339

Chain of upstream PRs as of 2025-11-23

alias_method_chain is deprecated in Rails 5.0 and removed in Rails 5.1,
so we need to stop using it. I can't find any way these are being
called, unless it ends up being ActiveRecord implict nonsense I can't
find evidence of.

Copy link
Contributor Author

bagedevimo commented Jul 4, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Holmes98
Copy link
Member

Holmes98 commented Jul 6, 2025

alias_method_chain :contest=, :update basically seems to be an shorthand for

alias_method :contest_without_update=, :contest=
alias_method :contest=, :contest_with_update=

So it was being used to modify the behaviour of contest=, while preserving access to the original method through contest_without_update=. But I think since we aren't using contest_without_update=, we can just directly override contest= instead? I've pushed a potential fix in a9ea1d7.

@bagedevimo bagedevimo force-pushed the remove-contest-finish-at-with branch from d69f5b5 to 28e2398 Compare July 19, 2025 06:30
@bagedevimo bagedevimo changed the base branch from master to graphite-base/315 July 19, 2025 06:30
@bagedevimo bagedevimo changed the base branch from graphite-base/315 to before-action-cont July 19, 2025 06:30
@bagedevimo bagedevimo force-pushed the remove-contest-finish-at-with branch 3 times, most recently from d68c051 to 3680297 Compare July 19, 2025 06:33
@bagedevimo bagedevimo marked this pull request as ready for review July 19, 2025 06:34
@coveralls
Copy link

coveralls commented Jul 19, 2025

Coverage Status

coverage: 37.642% (-0.01%) from 37.654%
when pulling 3725dc6 on remove-contest-finish-at-with
into 11c399d on migration-version.

@bagedevimo bagedevimo changed the base branch from before-action-cont to graphite-base/315 July 19, 2025 09:19
@bagedevimo bagedevimo force-pushed the remove-contest-finish-at-with branch from 3680297 to 317e3b4 Compare July 19, 2025 09:20
@graphite-app graphite-app bot changed the base branch from graphite-base/315 to master July 19, 2025 09:20
@bagedevimo bagedevimo force-pushed the remove-contest-finish-at-with branch from 317e3b4 to a4c34cf Compare July 19, 2025 09:20
@bagedevimo bagedevimo changed the base branch from master to graphite-base/315 July 20, 2025 09:20
@bagedevimo bagedevimo force-pushed the remove-contest-finish-at-with branch from a4c34cf to 5e8ee49 Compare July 20, 2025 09:20
@bagedevimo bagedevimo changed the base branch from graphite-base/315 to migration-version July 20, 2025 09:21
alias_method_chain is deprecated in Rails 5.0 and removed in Rails 5.1,
so we need to stop using it.

This allowed us to modify the behaviour of `ContestRelation#contest=`
while still providing access to the original method through
`contest_without_update=`. But since we weren't using that method
anyway, we can just directly override `contest=` instead.

Co-authored-by: Jonathan Khoo <jkhoo98@gmail.com>
@bagedevimo bagedevimo force-pushed the remove-contest-finish-at-with branch from 5e8ee49 to 3725dc6 Compare November 23, 2025 06:13
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.

3 participants