Skip to content

Improve docs and xrefs re tree building #726

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

Merged
merged 3 commits into from
Apr 1, 2023

Conversation

ijackson
Copy link
Contributor

I had to discover some of the facts here empirically so I thought I would write them up for other folks :-).

src/build.rs Outdated
/// Paths passed to `remove` and `upsert` can be multi-component paths, ie they
/// may contain slashes.
///
/// This is the more-cooked tree update facility. There is also [`TreeBuilder`]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// This is the more-cooked tree update facility. There is also [`TreeBuilder`]
/// This is a higher-level tree update facility. There is also [`TreeBuilder`]

(Parallel construction with "lower-level".)

@joshtriplett
Copy link
Member

I posted one very minor wording tweak, but otherwise this seems reasonable.

Would you consider adding coverage of the main limitation of the high-level API as well? Quoting the libgit2 documentation:

  • Deleting and adding the same entry is undefined behaviour, changing
  • a tree to a blob or viceversa is not supported.

ijackson and others added 3 commits April 1, 2023 13:27
It seems obvious from the rest of the docs what to do with a
`TreeUpdateBuilder`.  The reference to `git_tree_create_updated` is
certainloy not helpful since that's a C function.

Signed-off-by: Ian Jackson <[email protected]>
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

I went ahead and applied Josh's suggestion. Thanks!

@ehuss ehuss merged commit 838760a into rust-lang:master Apr 1, 2023
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