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

Re-organization of crates #525

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

mathias-arm
Copy link
Contributor

@mathias-arm mathias-arm commented Sep 28, 2022

This PR moves all Veracruz crates in crates/ directory (except for ones under workspaces).

@mathias-arm mathias-arm force-pushed the pr/crates-reorg branch 2 times, most recently from 44a3f3a to a966b4d Compare September 28, 2022 21:39
@mathias-arm mathias-arm force-pushed the pr/crates-reorg branch 2 times, most recently from 92c5201 to 0ee4eb8 Compare October 10, 2022 20:24
@mathias-arm
Copy link
Contributor Author

Will rebase after #651 is merged.

@mathias-arm mathias-arm force-pushed the pr/crates-reorg branch 4 times, most recently from d3439cc to 6fa7860 Compare August 15, 2023 20:51
@mathias-arm mathias-arm marked this pull request as ready for review August 15, 2023 20:51
@egrimley-arm
Copy link
Contributor

egrimley-arm commented Aug 24, 2023

I think the reasons for doing this should be recorded in the PR or in the log message. (Was there something about making grep easier?)

If I've understood correctly, this increases the number of symbolic links from 7 to 8 (by adding crates/third-party -> ../third-party) but gets rid of the potential recursion because we now have workspaces/*/crates -> ../../crates instead of workspaces/*/crates -> ../...

What disruption might be caused by moving third-party into the crates/ directory instead of adding that symbolic link? I can imagine that there might be problems with merging or rebasing when submodules move around, but I can't remember trying it so perhaps it just works?

On second thoughts, perhaps it's better to keep third-party/ at the top level anyway. But then I have to ask myself why QCBOR and t_cose are not in third-party/.

@mathias-arm
Copy link
Contributor Author

I think the reasons for doing this should be recorded in the PR or in the log message. (Was there something about making grep easier?)

Yes, I wanted to put the Veracruz Rust code in one place. Having the different crates at the top-level with others directories (in particular workspaces) is making things harder to grep for things.

What disruption might be caused by moving third-party into the crates/ directory instead of adding that symbolic link? I can imagine that there might be problems with merging or rebasing when submodules move around, but I can't remember trying it so perhaps it just works?

Initially I was thinking about moving third-party as well, but moving submodules adds a little bit of friction: when switching between branches before and after the move, git does not clean-up. Not a show-stopper though.

On second thoughts, perhaps it's better to keep third-party/ at the top level anyway.

I am now leaning also in this direction since it does help separating our code from third-party.

But then I have to ask myself why QCBOR and t_cose are not in third-party/.

That could also be addressed (later?).

@mathias-arm mathias-arm merged commit 785c974 into veracruz-project:main Aug 29, 2023
6 checks passed
@mathias-arm mathias-arm deleted the pr/crates-reorg branch August 29, 2023 14:08
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