Skip to content

Drop the benchmark dependency - #2407

Merged
amomchilov merged 1 commit into
Shopify:mainfrom
Earlopain:drop-benchmark-dep
Sep 15, 2026
Merged

amomchilov merged 1 commit into
Shopify:mainfrom
Earlopain:drop-benchmark-dep

Conversation

@Earlopain

Copy link
Copy Markdown
Contributor

Motivation

tapioca currently depends on the benchmark gem because of a change to Ruby bundled gems. The functionality is trivial though, and I don't think you have to take on a dependency for that.

Implementation

Just use Process.clock_gettime directly.

Tests

No behaviour change. I just ran some cli commands and they still print the time in seconds. Probably the output is also already asserted against somewhere.

@Earlopain
Earlopain requested a review from a team as a code owner October 16, 2025 10:23
@st0012 st0012 added the chore label Oct 16, 2025
@st0012

st0012 commented Oct 16, 2025

Copy link
Copy Markdown
Member

Thanks for the PR. Can you fix typechecking errors?

@Earlopain

Copy link
Copy Markdown
Contributor Author

Oops, yeah

@st0012

st0012 commented Oct 22, 2025

Copy link
Copy Markdown
Member

We have addressed a few CI issues this week, which should make this PR's build green. Can you rebase the PR? Thx

@st0012 st0012 self-assigned this Jan 6, 2026
@st0012

st0012 commented Jan 6, 2026

Copy link
Copy Markdown
Member

Based on CI failures, it looks like some dev dependencies Tapioca relies on (identity_cache, rails for example) uses benchmark but doesn't declare it as a dependency (at least not in the released versions Tapioca currently uses).
I think we'd have to wait for them to update their dependencies, cut a release, and bump them in Tapioca, before we can merge this one.

@Earlopain

Copy link
Copy Markdown
Contributor Author

How about now?

@KaanOzkan
KaanOzkan requested a review from a team September 15, 2026 15:03
@Morriar

Morriar commented Sep 15, 2026

Copy link
Copy Markdown
Contributor
Load Error is: cannot load such file -- benchmark

Looks like something needs to be fixed on the tests side.

@Earlopain
Earlopain force-pushed the drop-benchmark-dep branch 2 times, most recently from 102b942 to d97196d Compare September 15, 2026 15:59
It is trivial, don't have to take on a dependency for that.

The test change is because activesupport 7.1.0 itself doesn't depend on `benchmark` properly
@Earlopain

Copy link
Copy Markdown
Contributor Author

Right, some test requires real rails (and activesupport) 7.1.0 which doesn't declare the dependency. I bumped the test to the latest patch release and that should resolve it

@amomchilov
amomchilov merged commit 43bcbbc into Shopify:main Sep 15, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants