-
Notifications
You must be signed in to change notification settings - Fork 0
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
9-publish to PyPi #15
Conversation
Adbpy 9: add github action for release
…nsicInstitute/adb_py
contents: read | ||
|
||
jobs: | ||
# TODO issue #11: deduplicate this, and trigger the test.yml directly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not very happy with duplicating test.yml here, but I couldn't get it to work and did not want to spend more time on this, so created a separate issue
PYTHONPATH: ${{ steps.modify-sys-path.outputs.sys-path }} | ||
|
||
publish: | ||
needs: test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sure the publish
job is only ran after test
was finished and succeeded
.github/workflows/publish.yml
Outdated
- name: Build package | ||
run: python -m build | ||
- name: Publish package | ||
uses: pypa/gh-action-pypi-publish@27b31702a0e7fc50959f5ad993c78deac1bdfc29 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is 27b31702a0e7fc50959f5ad993c78deac1bdfc29 representing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the commit hash of this action. You can also put the tag or branch name here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this specific commit hash of this action, why not a specific release like is suggested on their github?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generated this script from the GitHub UI, and this was in there. If you prefer a release that's fine by me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it to the latest release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding this file closes #3
CONTRIBUTING.md
Outdated
|
||
By contributing to this project, you agree that your contributions will be licensed under the project's [LICENSE](LICENSE.txt). | ||
|
||
## Releasing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
releasing is not part of contributing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes good point, what is a better place for this information. I wanted to have it somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make a RELEASE_GUIDELINES.md
Additionally, I suggest to owners of the repository to set a minimum of approvals required before the pull request can be merged. (in the repository settings) |
Co-authored-by: WJ-van-Hoek <[email protected]>
I thought I did that, but I set it for main. Now this PR is targeted at main_new, the rule doesnt apply. I will add a rule for any branch |
CONTRIBUTING.md
Outdated
|
||
- Creating a release in GitHub automatically triggers publication on PyPI. Please ensure that your changes are properly documented in the release notes. | ||
- Semantic versioning should be used | ||
- The tag should be of the form v{version}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The tag should be of the form v{version}. | |
- The tag should adhere to the following format: `v{version}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Made some small text style suggestions.
Maybe squash the commits before merging? |
Co-authored-by: Carly-1 <[email protected]>
Co-authored-by: Carly-1 <[email protected]>
…sicInstitute/adb-pywrapper into 9-publish
Hmm let's discuss this in the team first. Normally we do not do that, but given this is an open source project we might want to have different rules. For now I will not squash them |
Publish adbpy-wrapper to PyPi. See https://pypi.org/project/adb-pywrapper
In order to achieve this the following things needed to be done:
I initially committed everything to main, which I temporarily allowed, because otherwise the github action would not be visible. By doing this, I made a bit of a mess of main experimenting with the workflow so I created a main_new which goes back to the commit before I started working on this. I propose to merge this to main_new and then remove the old main and rename main_new to main. All other commits are in this branch, so we will not lose any commits.