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] Move burn crates to their own crates directory #1336

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

syl20bnr
Copy link
Member

@syl20bnr syl20bnr commented Feb 20, 2024

Pull Request Template

Checklist

  • Confirmed that run-checks all script has been executed.
  • Made sure the book is up to date with changes in this PR.

Related Issues/PRs

Fixes #1263

Changes

Move all the burn crates to their dedicated directory called crates.
We chose to keep the crate folder names to be the same as the actual crate name, see discussed in #1263

I'll update the links in the contributor book once this is merged.

Testing

run-checks and making sure our CI passes.

Copy link

codecov bot commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e9bb273) 84.49% compared to head (b77184a) 78.77%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1336      +/-   ##
==========================================
- Coverage   84.49%   78.77%   -5.72%     
==========================================
  Files         571      551      -20     
  Lines       63818    61836    -1982     
==========================================
- Hits        53923    48712    -5211     
- Misses       9895    13124    +3229     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@syl20bnr syl20bnr force-pushed the refactor/move-burn-crates branch 2 times, most recently from 58a81f2 to 9c61841 Compare February 20, 2024 16:11
@syl20bnr syl20bnr marked this pull request as ready for review February 20, 2024 16:13
@syl20bnr syl20bnr force-pushed the refactor/move-burn-crates branch from 9c61841 to 8da91f9 Compare February 20, 2024 16:18
@Luni-4
Copy link
Collaborator

Luni-4 commented Feb 20, 2024

Fantastic! Perhaps we can add

members = [
  "crates/*",
]

to the main Cargo.toml instead of each single crate

I was wondering if it would be clearer to move examples crates, since they are independent, into a new burn repository such as the models repository.

@syl20bnr syl20bnr force-pushed the refactor/move-burn-crates branch from 8da91f9 to b77184a Compare February 20, 2024 18:18
@syl20bnr
Copy link
Member Author

I updated the Cargo file with crates/*. 👍

We have no mechanism to publish examples alongside versions and they are using local crates from the repo. So for now it is better to keep them in the monorepo.

@antimora
Copy link
Collaborator

Fantastic! Perhaps we can add

members = [
  "crates/*",
]

to the main Cargo.toml instead of each single crate

I was wondering if it would be clearer to move examples crates, since they are independent, into a new burn repository such as the models repository.

I would add we should keep in the same repository. It would be difficult to synchronize the versions

@Luni-4
Copy link
Collaborator

Luni-4 commented Feb 20, 2024

To synchronize the versions we just have to change from path to git since examples do not have to be published it should not be problematic moving them in another repository, but ok, no problem in leaving them here

@syl20bnr syl20bnr merged commit 4427768 into main Feb 20, 2024
15 checks passed
@syl20bnr syl20bnr deleted the refactor/move-burn-crates branch February 20, 2024 18:57
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.

Refactor: move all crates to crates folder
4 participants