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

Release guide #820

Merged
merged 5 commits into from
Apr 18, 2022
Merged

Conversation

LysandreJik
Copy link
Member

This PR adds a small release guide to the setup.py for releases.

I added it here as it's the same location as for the transformers library and it seems adequate, but happy to move it somewhere else more appropriate.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Apr 5, 2022

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

@LysandreJik
Copy link
Member Author

Following the guide to do a patch, I realize there's some info missing. Will complete once I'm done with the patch.

Copy link
Contributor

@adrinjalali adrinjalali 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 @LysandreJik . I think it'd be nice to move this to doc/dev/release.mdx rather than having it in setup.py.

setup.py Outdated Show resolved Hide resolved
setup.py Outdated
Comment on lines 20 to 24
4. Make sure that the conda build works correctly by building it locally:
```
conda install -c defaults anaconda-client conda-build
HUB_VERSION=<VERSION> conda-build .github/conda
```
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to maintain conda here? transformers doesn't seem to be uploading to our channel anymore, and we have our package on conda-forge. That recipe is more up-to-date.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now the build works well and is automatic; I'd like to draft a small message to share with the rest of the team regarding conda support and whether we want to continue to support it through our own channel or not. In the meantime, I'd like to keep it here.

setup.py Outdated
Comment on lines 33 to 38
6. Commit, tag, and push the branch:
```
git commit -am "Release: v<VERSION>"
git tag v<VERSION> -m "Adds tag v<VERSION> for pypi and conda"
git push -u --tags origin v<MINOR-VERSION>-release
```
Copy link
Contributor

Choose a reason for hiding this comment

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

to give visibility to others, this can be a PR to the release branch instead of direct commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would also help with checking the doc build on that PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

How would you name the branch from which that PR is made? Others will get visibility with the release already, I am not 100% sure I see why the PR is necessary here

Copy link
Contributor

Choose a reason for hiding this comment

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

that PR comes from the maintainer's own fork, doesn't matter how it's named. E.g: scikit-learn/scikit-learn#21120

In most cases it'll be a simple PR, but sometimes you might want to include more commits from the main branch.

setup.py Outdated
Comment on lines 43 to 46
8. Checkout main once again to update the version in the `__init__.py` file:
```
git checkout main
```
Copy link
Contributor

Choose a reason for hiding this comment

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

this can also be a PR

LysandreJik and others added 2 commits April 12, 2022 19:19
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Copy link
Contributor

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

This is a good start anyway. I'm happy for this to be merged, updated or not :)

@LysandreJik
Copy link
Member Author

I'm having a hard time finding coherent names for the branches as I haven't followed that workflow previously; I'll merge this as it is, but very open if you want to open another PR to complete the existing doc with the two additional branches :)

@LysandreJik LysandreJik merged commit 18e6e87 into huggingface:main Apr 18, 2022
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.

None yet

3 participants