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.

Sign up for GitHub

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

Jump to bottom

✨ parent_commit argument for create_commit and related functions #916

Merged
merged 11 commits into from
Aug 11, 2022
Merged

✨ parent_commit argument for create_commit and related functions #916

merged 11 commits into from
Aug 11, 2022

Conversation

SBrandeis
Copy link
Contributor

@SBrandeis SBrandeis commented Jun 20, 2022 •
edited
Loading

... And improve the documentation for create_commit

@SBrandeis SBrandeis force-pushed the create-commit-parent-commit branch from b0333f6 to 08b11a0 Compare June 20, 2022 12:53
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jun 20, 2022 •
edited
Loading

The documentation is not available anymore as the PR was closed or merged.

@SBrandeis SBrandeis changed the title 📝 Improve documentation quality for create_commit ✨ parent_commit argument for create_commit and related functions Jun 20, 2022
@SBrandeis
Copy link
Contributor Author

cc @coyotte508

@SBrandeis SBrandeis marked this pull request as ready for review June 20, 2022 13:53
@SBrandeis SBrandeis requested review from LysandreJik and osanseviero and removed request for osanseviero and LysandreJik June 20, 2022 13:54
LysandreJik
LysandreJik approved these changes Jun 29, 2022
View reviewed changes
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for working on the implementation @SBrandeis!

src/huggingface_hub/hf_api.py Outdated Show resolved Hide resolved
src/huggingface_hub/hf_api.py Outdated Show resolved Hide resolved
@SBrandeis @LysandreJik
Update src/huggingface_hub/hf_api.py
de61c8b 
Co-authored-by: Lysandre Debut <lysandre@huggingface.co>
@SBrandeis SBrandeis requested a review from LysandreJik July 26, 2022 12:24
@LysandreJik
Copy link
Member

One thing I would maybe clarify in the docs is the purpose of the parent_commit parameter. I understand its purpose easily in the create_commit API, less-so in the upload_file/delete_file. I think having example usage of this argument would be quite important in order to understand what it does and how to use it.

I think the "create_commit API" header could also be beefed-up a little in terms of documentation and understandability, there isn't a lot there and we dive straight into the classes themselves without understanding their purpose and if users should use them or not.

I'm happy for this to be taken care of in another PR if you'd like to have this in first.

osanseviero
osanseviero approved these changes Jul 27, 2022
View reviewed changes
Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

src/huggingface_hub/hf_api.py Outdated Show resolved Hide resolved
@SBrandeis SBrandeis requested a review from Wauplin August 4, 2022 15:45
@Wauplin Wauplin mentioned this pull request Aug 10, 2022
Closed
@Wauplin
Copy link
Contributor

Wauplin commented Aug 10, 2022

I created this issue #984 to improve documentation. Will close this PR now.

@Wauplin Wauplin merged commit 9eff0e9 into huggingface:main Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coyotte508 coyotte508 coyotte508 left review comments

@Wauplin Wauplin Wauplin approved these changes

@osanseviero osanseviero osanseviero approved these changes

@adrinjalali adrinjalali Awaiting requested review from adrinjalali

@julien-c julien-c Awaiting requested review from julien-c

@LysandreJik LysandreJik Awaiting requested review from LysandreJik

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@SBrandeis @HuggingFaceDocBuilderDev @LysandreJik @Wauplin @coyotte508 @osanseviero