Conversation
|
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. |
| Instead of committing directly, this opens a pull request on the Hub for | ||
| each branch that has changes: one against `main` for the kernel card and | ||
| one against the `v<version>` branch for the build artifacts and | ||
| benchmarks. The pull request URLs are printed after the upload, so that | ||
| the repository maintainers can be pointed to them for review. |
There was a problem hiding this comment.
Happy to also scope it to just version branches if we want.
|
|
||
| repo.create_commit() | ||
| let open_pr = args.create_pr && pr_revision.is_none(); | ||
| let revision = pr_revision.clone().unwrap_or_else(|| branch.clone()); |
There was a problem hiding this comment.
Meh, it's annoying that hf-hub doesn't take &str or impl ToOwned, forcing all this work on the caller. Could be a much cleaner pr_revision.unwrap_or(&branch) otherwise.
Maybe worth raising an issue?
Co-authored-by: Daniël de Kok <me@danieldk.eu>
The "Apply suggestions from code review" commit left two blocks with mismatched delimiters and a nested Option: - repo_id extraction lost its `and_then` flatten and `unwrap_or(repo_id)` fallback, producing `Option<Option<String>>`. - branch-creation error handling had unbalanced parens/braces and dropped the `?` propagation. Restore both so the crate compiles; fixes Clippy, Rustfmt, Test, check-cli-docs, and build jobs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
drbh
left a comment
drbh
left a comment
There was a problem hiding this comment.
looking good, just left some comments/nits
| let repo_id = repo_url | ||
| .url | ||
| .trim_end_matches('/') | ||
| .strip_prefix("https://huggingface.co/") | ||
| .map(|s| s.strip_prefix("kernels/").unwrap_or(s)) | ||
| .unwrap_or(&repo_id) | ||
| .to_owned(); | ||
| .as_ref() | ||
| .and_then(|repo_url| { | ||
| repo_url | ||
| .url | ||
| .trim_end_matches('/') | ||
| .strip_prefix("https://huggingface.co/") | ||
| .map(|s| s.strip_prefix("kernels/").unwrap_or(s).to_owned()) | ||
| }) | ||
| .unwrap_or(repo_id); |
There was a problem hiding this comment.
maybe we can avoid changing this logic by skipping the repo url logic all together when the create pr flag is used. I think we are only operating on the repo_url response in the case the repo creation returned a sanitized repo id.
since we shouldn't create a new repo when the create pr flag is used maybe when the flag exists we simply use the provided repo id?
There was a problem hiding this comment.
I believe this should have been addressed by now.
4 participants
Example: https://huggingface.co/kernels/sayakpaul/activation-create-pr-test/discussions/1
In the subsequent PR, I will push modifications to
kernels-communityCI so that PRs are raised for kernels like https://huggingface.co/kernels/MiniMaxAI/msa