Skip to content

Conversation

@Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Oct 4, 2024

I first started to work on this PR to document how to integrate a library with the Hub (i.e. how to open the PR on huggingface.js). In the end, I've updated more than that to focus the page on "how to implement the integration?" + "how to register your library?".

I feel that some sections are outdated, typically the "how to login" (out of scope for a library integration?) and "how to add inference API" (we don't want to add new libs to api-inference-community, right?) so I removed them. Let me know if you feel strongly against it.

Once merged, I will open a PR to delete https://github.com/huggingface/huggingface.js/blob/main/.github/pull_request_template/new_library.md which is currently useless (not discoverable 😕)


For reviewers: https://moon-ci-docs.huggingface.co/docs/hub/pr_1442/en/models-adding-libraries

@Wauplin Wauplin requested review from osanseviero and pcuenca October 4, 2024 11:49
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

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.

Looking very nice!

@Wauplin Wauplin requested a review from osanseviero October 7, 2024 07:54
@Wauplin Wauplin requested review from Vaibhavs10 and julien-c October 7, 2024 08:02
Copy link
Member

@Vaibhavs10 Vaibhavs10 left a comment

Choose a reason for hiding this comment

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

Niceee! 🔥 - Read through the PR didn't find any grammatical errors and such. Love the overall flow of the doc as well.

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.

Nice! 🔥

}
```
3. Finally, run the following test to ensure everything works as expected:
Here is a minimal [example](https://github.com/huggingface/huggingface.js/pull/561/files) adding integration for Grok-1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have an example including a snippet and a simpler download rule?

Copy link
Member

Choose a reason for hiding this comment

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

What about this one? https://github.com/huggingface/huggingface.js/pull/885/files - It looks quite clean and has download counts and snippets both.

Copy link
Member

Choose a reason for hiding this comment

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

Created a suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! The VFI Mamba PR is indeed very nice with a complete PR and still concise. I've merged your suggestiong @Vaibhavs10 + updated the second link (Grok-1 PR was referenced twice). We should be good now!

@Wauplin
Copy link
Contributor Author

Wauplin commented Oct 8, 2024

Thanks for the reviews ❤️ Let's get this merged and hopefully see some high-quality PRs flowing in 😄

@Wauplin Wauplin merged commit 33579c3 into main Oct 8, 2024
2 checks passed
@Wauplin Wauplin deleted the docs-models-adding-libraries branch October 8, 2024 13:39
Wauplin added a commit to huggingface/huggingface.js that referenced this pull request Oct 9, 2024
Reverting #913 now
that huggingface/hub-docs#1442 has been merged.
The PR template was not discoverable at all. Now users should be
redirected to https://huggingface.co/docs/hub/models-adding-libraries
which contains the same information.
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.

5 participants