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.

Sign up for GitHub

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

Jump to bottom

Ordinal methods should respect loaded records #31006

Merged
merged 1 commit into from Nov 25, 2017
Merged

Ordinal methods should respect loaded records #31006

merged 1 commit into from Nov 25, 2017

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Oct 28, 2017

We should reset partially loaded @offsets cache when latest records
has loaded because the cache has been staled and it may not be
consistent with latest records.

@kamipo
Ordinal methods should respect loaded records
cd4cbfc 
We should reset partially loaded `@offsets` cache when latest records
has loaded because the cache has been staled and it may not be
consistent with latest records.
@kamipo kamipo mentioned this pull request Oct 28, 2017
Closed
Schwad
Schwad approved these changes Nov 9, 2017
View reviewed changes
Copy link
Contributor

@Schwad Schwad left a comment

Choose a reason for hiding this comment

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

This looks fine and makes sense to me. What do you think @matthewd ?

@eileencodes eileencodes merged commit 0f79ab9 into master Nov 25, 2017
@matthewd
Copy link
Member

Sorry I didn't comment on this earlier 😔

The description doesn't make it obvious, but this philosophically conflicts with #30800. I still think that one's the way to go, but for now I think we should hold off on either until we can find some consensus on which behaviour is the more desirable.

@eileencodes
Copy link
Member

Reverted in d7ab571 due to @matthewd's comment

eileencodes added a commit that referenced this pull request Nov 26, 2017
@eileencodes
Revert "Merge pull request #31006 from rails/kamipo/ordinal_methods_s…
d7ab571 
…hould_respect_loaded_records"

This reverts commit 0f79ab9, reversing
changes made to d575f7f.

This PR philosophically conflicts with #30800 and Matthew thinks we
should hold off merging this until we find concensus. Reverting since
we're about to cut a release for 5.2.
@kamipo kamipo deleted the kamipo/ordinal_methods_should_respect_loaded_records branch November 26, 2017 23:49
@kamipo kamipo mentioned this pull request Jun 24, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Schwad Schwad Schwad approved these changes

@matthewd matthewd Awaiting requested review from matthewd

Assignees
No one assigned
Labels
activerecord
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@kamipo @matthewd @eileencodes @Schwad