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

Replace obsolete function argument name in docs for api.create_repo #795

Closed
wants to merge 1 commit into from

Conversation

pavel-lexyr
Copy link

repo_id does not appear to work. Is name the newer version?

`repo_id` does not appear to work. Is `name` the newer version?
@pavel-lexyr pavel-lexyr changed the title Replace obsolete function argument name Replace obsolete function argument name in docs for api.create_repo Mar 23, 2022
@adrinjalali
Copy link
Contributor

Hi @pavel-lexyr , what makes you think name is the new version? name and organization are deprecated in favor of repo_id.

@osanseviero
Copy link
Contributor

I do think it's an issue since the change has not been released but the docs were updated as in https://huggingface.co/docs/hub/adding-a-model#creating-a-repository-2

@osanseviero osanseviero reopened this Mar 24, 2022
@LysandreJik
Copy link
Member

Ah, good catch! I wasn't aware that the docs were automatically deployed.

This will change with #782 as the docs will be deployed on a version-basis to the stable docs, and the docs for main will be pushed to the main version of the docs (non-default).

@osanseviero
Copy link
Contributor

osanseviero commented Mar 24, 2022

The docs are updated when they are hub-docs sub-module is updated in moon landing

@adrinjalali
Copy link
Contributor

So what's the path forward here? I don't think we want to merge this to main.

@LysandreJik
Copy link
Member

@osanseviero mentioned that he would like to see the following PRs merged before the next release:

I personally would like to see #782 merged before the next release.

We can either:

  • Release v0.5.0 right now to update the public facing API, and release v0.6.0 once all of these PRs have been merged (probably in ~1 week?). Downside to this is that we do two very fast consecutive releases. Not too bad given that we don't have a written statement of current timing between releases, and since we're not at v1.0 yet deprecation cycles between minor releases are theoretically nice-to-haves, even if we're very attached to them.
  • Focus on merging the PRs above and release v0.5.0 then. We'll likely have an additional few days where the documentation is incorrect (but correct for the source install), but I personally think that's okay. It should be the last occurrence of such situations.

What do you think, @osanseviero, @adrinjalali?

@osanseviero
Copy link
Contributor

IMO none of the PRs I mentioned are release-blocking so it is ok if we do two releases so our docs are in sync. We have also done a lot of interesting work since last release, so I think we can go ahead 😄

No strong opinion though.

@adrinjalali
Copy link
Contributor

I'd rather wait for those PRs (maybe except #782, since I'm not sure how much work that is. It looks like that PR's gonna take a while to be merged).

I think we should have a slow-ish release cycle for this library since it's the core library used by all integrations, and the deprecation cycle should give people enough time to adapt their code. Having two releases in the span of a week doesn't give people enough time to adapt.

@osanseviero
Copy link
Contributor

SGTM!

@LysandreJik
Copy link
Member

This can be closed as the new release was done and repo_id is now the recommended way to handle it! Thanks for your PR @pavel-lexyr, and sorry for the inconvenience!

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.

4 participants