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

Cargo design doc #441

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

Conversation

brunoapimentel
Copy link
Contributor

@brunoapimentel brunoapimentel commented Jan 3, 2024

A design doc for implementing Cargo support. It mostly follows the general principle for other package managers. The bigger issue of having Rust-based dependencies is described in an appendix since this is not strictly Rust support.

Corresponding repository for ITs is https://github.com/cachito-testing/cachi2-rust

Maintainers will complete the following section

  • Commit messages are descriptive enough
  • Code coverage from testing does not decrease and new code is covered
  • Docs updated (if applicable)
  • Docs links in the code are still valid (if docs were updated)

@brunoapimentel brunoapimentel force-pushed the cargo-design-doc branch 3 times, most recently from 6057428 to 5cd372b Compare January 4, 2024 00:48
@eskultety
Copy link
Member

it'll be a while before I get to read/understand/review this, but kudos for pushing the "PR for designs" way. Feels like it's going to be more ergonomic + we can not only close the PR, but also retain the doc for future reference, OR convert it to docs, i.e. it gives us a bit more flexibility.

@eskultety eskultety added the design document A new feature design (blueprint) label Jan 4, 2024
@eskultety eskultety mentioned this pull request Aug 22, 2024
5 tasks
@a-ovchinnikov a-ovchinnikov force-pushed the cargo-design-doc branch 2 times, most recently from 8ec2e18 to 3c48165 Compare January 21, 2025 13:11
@slimreaper35
Copy link
Member

Is this ready for review ?

@a-ovchinnikov
Copy link
Contributor

Is this ready for review ?

All major parts are there, I will still be tweaking the document a little, but yes, it is ready for review/suggestions.

@a-ovchinnikov a-ovchinnikov marked this pull request as ready for review January 21, 2025 16:02
@a-ovchinnikov a-ovchinnikov force-pushed the cargo-design-doc branch 3 times, most recently from d243406 to 9e8e42a Compare January 22, 2025 21:57
@brunoapimentel
Copy link
Contributor Author

Now that we have ADRs in the repo, I wonder what approach we should take with these design docs. Should they just become ADRs? I feel like they're too big for an ADR, but the info here is important. Wdyt?

docs/designs/cargo-support.md Outdated Show resolved Hide resolved
docs/designs/cargo-support.md Outdated Show resolved Hide resolved
docs/designs/cargo-support.md Outdated Show resolved Hide resolved
docs/designs/cargo-support.md Outdated Show resolved Hide resolved
@slimreaper35
Copy link
Member

slimreaper35 commented Jan 23, 2025

The path for the document is slightly incorrect.
s/docs/designs/cargo-support.md/docs/design/cargo.md (in case this ends up being a design document)

@a-ovchinnikov
Copy link
Contributor

Now that we have ADRs in the repo, I wonder what approach we should take with these design docs. Should they just become ADRs? I feel like they're too big for an ADR, but the info here is important. Wdyt?

I think keeping them in design directory is fine. I am not sure if formalizing them more would actually help -- there are not too many decisions and designs to require them to strictly follow the same pattern (and there is no set rule for a pattern anyway).

Copy link
Member

@eskultety eskultety left a comment

Choose a reason for hiding this comment

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

This is an amazing write-up! I skipped the Python related parts since I don't think those parts are heavily dependent on which approach for prefetching we end up selecting, so I'd probably prefer to save that information for a different doc/ADR and make this one leaner.
I raised a few questions to help me better understand the background.

docs/design/cargo-support.md Show resolved Hide resolved
docs/design/cargo-support.md Show resolved Hide resolved
docs/design/cargo-support.md Outdated Show resolved Hide resolved
docs/design/cargo-support.md Show resolved Hide resolved
Comment on lines +643 to +664
## Appendix A. Rust extensions to Python

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if these Python related appendices should be part of this design doc/ADR in order to be merged. I perceive that work to be logically connected, but not imperative to deliver the functionality for what is discussed here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is only tangentially related to Rust support, so there is a good argument for dropping it. However to the best of my knowledge the original request was more about supporting Rust extensions to Python than just supporting Rust itself, so I think it is ok to keep it as an Appendix.

Copy link
Member

Choose a reason for hiding this comment

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

the original request was more about supporting Rust extensions to Python than just supporting Rust itself

That is true, however, this work is the initial step towards resolving that

, so I think it is ok to keep it as an Appendix.

On one hand I don't want to lose the information because it needs to be added to the follow up work, on the other hand though these appendices just bloat the document with only tangentially related information. What's the plan for adding a design doc on the follow up work, will it contain duplicate information, will the information here in appendices be transferred over to the new document or will we just link it from the new docs? Again, I don't question the depth or quality of information provided, just the timeline and the expected outcome.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think transferring the Appendices is the best approach: by being appendices to a design they will be preserved and also out of sight, then once we are ready to work on Python-Rust support we can move them out and put them into a separate document.

@a-ovchinnikov
Copy link
Contributor

a-ovchinnikov commented Jan 30, 2025

This is an amazing write-up!

I must point out that it is 95% @brunoapimentel's work that I have inherited.

@a-ovchinnikov a-ovchinnikov force-pushed the cargo-design-doc branch 2 times, most recently from 6eda621 to 0e8ece5 Compare January 30, 2025 17:19
@eskultety
Copy link
Member

The strictly relevant parts LGTM. I have not invested time into reviewing the appendices.

This commit introduces Cargo supporting package manager design
and also contains information about the direction of future work
on support for Rust-based Python extensions.

Co-authored-by: Bruno FS Ciconelle <[email protected]>
Co-authored-by: Bruno Pimentel <[email protected]>
Signed-off-by: Alexey Ovchinnikov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design document A new feature design (blueprint)
Development

Successfully merging this pull request may close these issues.

4 participants