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(repo): move all crates into crates folder #1268

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

drahnr
Copy link
Contributor

@drahnr drahnr commented Feb 6, 2025

Moves all rust crates into a joint crates folder, binding is moved out from contracts into crates as contract-bindings, fendermint becomes client

This change is Reviewable

@drahnr drahnr changed the title Bernhard move all crates into crates folder refactor(repo): move all crates into crates folder Feb 6, 2025
@drahnr drahnr changed the base branch from main to bernhard-rename-extras-to-demo February 7, 2025 09:03
@drahnr drahnr force-pushed the bernhard-move-all-crates-into-crates-folder branch from 77e21bc to 4000d02 Compare February 7, 2025 09:09
Base automatically changed from bernhard-rename-extras-to-demo to main February 7, 2025 09:15
@drahnr drahnr marked this pull request as ready for review February 7, 2025 09:16
@drahnr drahnr requested a review from a team as a code owner February 7, 2025 09:16
@drahnr drahnr force-pushed the bernhard-move-all-crates-into-crates-folder branch from 4000d02 to 1edaed6 Compare February 7, 2025 09:16
Copy link
Contributor

@karlem karlem left a comment

Choose a reason for hiding this comment

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

  1. The Docker image build is failing.
  2. There are still references to "fendermint" in a few places in the codebase. The PR description should mention that we are renaming "fendermint" to "client."
  3. I would abstain from committing the generated bindings into Git and instead have them generated as part of the Cargo build process or something similar.

@drahnr drahnr force-pushed the bernhard-move-all-crates-into-crates-folder branch from 1edaed6 to 6e28ee7 Compare February 7, 2025 16:28
@drahnr drahnr force-pushed the bernhard-move-all-crates-into-crates-folder branch 4 times, most recently from 860bf6f to 7678f46 Compare February 10, 2025 08:12
@drahnr
Copy link
Contributor Author

drahnr commented Feb 10, 2025

There are still references to "fendermint" in a few places in the codebase. The PR description should mention that we are renaming "fendermint" to "client."

I cannot rename all of fendermint right now, since we still produce a binary named fendermint and APIs. This will be part of a larger type rename series, doing this now will hurt too much. I am also not sold on client, so if you think node is bettter, happy to adjust.

@raulk
Copy link
Contributor

raulk commented Feb 10, 2025

I think "client" is ambiguous because it's a relative term -- "a client of what?" Would prefer "node"

Copy link
Contributor

@karlem karlem left a comment

Choose a reason for hiding this comment

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

@drahnr I agree with the rename from client to node, but there are still some references to client left. I suggest searching for client/.

@drahnr drahnr force-pushed the bernhard-move-all-crates-into-crates-folder branch from 85ac7cb to f0c5edf Compare February 10, 2025 13:08
Copy link
Contributor

@karlem karlem left a comment

Choose a reason for hiding this comment

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

LGT, but the CI needs to be fixed first. Build is failing.

@drahnr drahnr force-pushed the bernhard-move-all-crates-into-crates-folder branch 4 times, most recently from a66df56 to 4a6461d Compare February 13, 2025 19:20
@drahnr drahnr force-pushed the bernhard-move-all-crates-into-crates-folder branch 3 times, most recently from 29ea3cd to 45b5148 Compare February 17, 2025 15:42
@drahnr drahnr force-pushed the bernhard-move-all-crates-into-crates-folder branch from 45b5148 to 9189a17 Compare February 19, 2025 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

4 participants