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

refactor: remove trin-utils dependency in ethportal-api #1581

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

njgheorghita
Copy link
Collaborator

What was wrong?

We can't release another ethportal-api version as long as it has a dependency on trin-utils

How was it fixed?

Extracted relevant "version" utils into ethportal-api

To-Do

@njgheorghita njgheorghita marked this pull request as ready for review November 4, 2024 18:54
Copy link
Member

@KolbyML KolbyML left a comment

Choose a reason for hiding this comment

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

:shipit:

@njgheorghita njgheorghita merged commit 8ba3dcf into ethereum:master Nov 4, 2024
9 checks passed
@njgheorghita njgheorghita deleted the relocate-version branch November 4, 2024 19:35
@carver
Copy link
Collaborator

carver commented Nov 4, 2024

The version of trin shows up incorrectly now:

cargo run -p trin -- --version
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.27s
     Running `target/debug/trin --version`
trin 0.3.0-6a02ed9f linux-x86_64 rustc1.81.0

(I guess this should be in a test, that the retrieved version for the CLI matches the trin crate version)

Potential solutions that seem suboptimal:

  • publish trin-utils
  • bind the version of ethportal-api and trin, to always release in lockstep (lots of problems with this. would we jump trin forward?)
  • make build_info look outside of the ethportal-api crate (maybe not possible? even if it is, probably not what we want anyway)

The best alternative I can see:
Remove any dependency inside ethportal-api on the current version of trin, and move the build info back to trin-utils (Requires some research. What do we give up by not setting the version in the #command macro on TrinConfig ?)

Without this decoupling the version from ethportal-api, then the reported version in trin will depend on ethportal-api's build. Which is weird and probably wrong too often.

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