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

refactor(store,sync)!: drop NewStoreWithHead #191

Merged
merged 1 commit into from
Jun 3, 2024
Merged

refactor(store,sync)!: drop NewStoreWithHead #191

merged 1 commit into from
Jun 3, 2024

Conversation

cristaloleg
Copy link
Contributor

Overview

Drop store.NewStoreWithHead. Replace it with a test helpers where appropriate. store package now has newStoreWithHead, sync has newTestStore. Both now require tb testing.TB as a 1st param which simplifies usage for the future tests and benchmarks.

Fixes #188

renaynay
renaynay previously approved these changes May 29, 2024
View reviewed changes
@cristaloleg cristaloleg changed the title feat(store,sync): drop NewStoreWithHead refactor(store,sync): drop NewStoreWithHead May 29, 2024
@cristaloleg cristaloleg changed the title refactor(store,sync): drop NewStoreWithHead !refactor(store,sync): drop NewStoreWithHead May 29, 2024
walldiss
walldiss reviewed May 30, 2024
View reviewed changes
Copy link
Member

@walldiss walldiss left a comment

Choose a reason for hiding this comment

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

newStoreWithHead and newTestStore seems very similar. Would it be possible to remove one in favour of another?

store/store_test.go Outdated Show resolved Hide resolved
@cristaloleg cristaloleg requested a review from ramin as a code owner May 31, 2024 08:30
@renaynay renaynay changed the title !refactor(store,sync): drop NewStoreWithHead refactor(store,sync)!: drop NewStoreWithHead Jun 3, 2024
@renaynay
Copy link
Member

renaynay commented Jun 3, 2024

Marking with break even though mostly "unused"

@renaynay renaynay merged commit cd70c84 into main Jun 3, 2024
4 checks passed
@renaynay renaynay deleted the store/drop-NewStoreWithHead branch June 3, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@walldiss walldiss walldiss left review comments

@renaynay renaynay renaynay approved these changes

@Wondertan Wondertan Wondertan approved these changes

@ramin ramin ramin approved these changes

@tzdybal tzdybal Awaiting requested review from tzdybal tzdybal is a code owner

@vgonkivs vgonkivs Awaiting requested review from vgonkivs vgonkivs is a code owner

@gupadhyaya gupadhyaya Awaiting requested review from gupadhyaya gupadhyaya is a code owner

Assignees
No one assigned
Labels
kind:break! kind:refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

store: drop NewStoreWithHead by adding an option to NewStore
5 participants
@cristaloleg @renaynay @ramin @walldiss @Wondertan