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.

Sign up for GitHub

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

Jump to bottom

update framework #77

Merged
merged 15 commits into from
Jul 19, 2023
Merged

update framework #77

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
28a176b
purge changelog
casperdcl Jul 5, 2023
13ce71f
minify licen[cs]e
casperdcl Jul 5, 2023
a57a443
migrate setup.py => pyproject.toml
casperdcl Jul 5, 2023
7e5d3bc
ci: update PyPI workflow
casperdcl Jul 5, 2023
0a356eb
deps: add qdarkstyle
casperdcl Jul 5, 2023
df36485
build: purge conda packaging
casperdcl Jul 5, 2023
28ee6bc
tests: migrate to pytest
casperdcl Jul 6, 2023
107cbd7
tests: add CI config
casperdcl Jul 6, 2023
339f16d
build: fix readme metadata
casperdcl Jul 6, 2023
c321b8c
minor tidy
casperdcl Jul 6, 2023
29cf08d
tests: DRY
casperdcl Jul 6, 2023
4ba9d8e
review comments
casperdcl Jul 6, 2023
4e918e6
build: enforce annotated tags
casperdcl Jul 17, 2023
756ea95
build: skip PyPI upload on existing version
casperdcl Jul 17, 2023
f3ca0dd
update URLs after repo migration
casperdcl Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions .github/workflows/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
# GitHub Actions

## Building the Conda Package: [conda_build_and_publish.yml](https://github.com/paskino/qt-elements/blob/main/.github/workflows/conda_build_and_publish.yml)
This github action builds and tests the conda package, by using the [conda-package-publish-action](https://github.com/paskino/conda-package-publish-action)
Runs automatically on every commit via [test.yml](./test.yml).

When pushing to main *all* variants are built and tested.
## Testing

When making an [annotated](https://git-scm.com/book/en/v2/Git-Basics-Tagging) tag, *all* variants are built, tested and published to the [paskino conda channel for qt-elements](https://anaconda.org/paskino/eqt/files). This package is noarch.
Runs `pytest`.

When opening or modifying a pull request to main, a single variant is built and tested, but not published. This variant is `python=3.7` and `numpy=1.18`.
## Building

## Building the PyPi Package: [pypi_publish.yml](https://github.com/paskino/qt-elements/blob/main/.github/workflows/pypi_publish.yml)
This github action builds the pypi package, by using the [deploy-pypi action](https://github.com/casperdcl/deploy-pypi).
Runs automatically after tests (above) succeed.

When pushing to main it is built and checked.
Builds binary (`*.whl`) & source (`*.tar.gz`) distributions.

When making an [annotated](https://git-scm.com/book/en/v2/Git-Basics-Tagging) tag, it is built and published to the [PyPi](https://pypi.org/project/eqt/#description).
## Releasing

Runs automatically -- when a [tag](https://git-scm.com/book/en/v2/Git-Basics-Tagging) is pushed -- after builds (above) succeed.

Publishes to [PyPI](https://pypi.org/project/eqt) and drafts changelog (release notes) at <https://github.com/paskino/qt-elements/releases>.

:warning: The draft notes above need to be manually tidied & approved by a maintainer.
27 changes: 0 additions & 27 deletions .github/workflows/conda_build_and_publish.yml
View file
Open in desktop

This file was deleted.

35 changes: 0 additions & 35 deletions .github/workflows/pypi_publish.yml
View file
Open in desktop

This file was deleted.

46 changes: 46 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: Test
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we could rename this and the filename to reflect that it both tests and possibly deploys?

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
name: Test
name: Test & Deploy

on:
push:
pull_request:
Comment on lines +3 to +4
Copy link
Member

Choose a reason for hiding this comment

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

Do we not need to specify the branch (main)?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, it's better not to

jobs:
test:
if: github.event_name != 'pull_request' || !contains('OWNER,MEMBER,COLLABORATOR', github.event.pull_request.author_association)
name: py${{ matrix.python }}
runs-on: ubuntu-latest
strategy:
matrix:
python: [3.7, 3.11]
Copy link
Collaborator

Choose a reason for hiding this comment

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

will the binaries built be only available for Python 3.7 and 3.11? In such a case, I'd like to be sure that these are available for all Python versions compatible with CIL, which are from 3.8 to 3.10!

Copy link
Member Author

Choose a reason for hiding this comment

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

No, binaries are built for all python versions. CI unit tests are only run for min & max supported versions.

steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python }}
- run: pip install -U .[dev]
- run: pytest
deploy:
needs: [test]
name: PyPI Deploy
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- uses: actions/setup-python@v4
with:
python-version: '3.x'
- id: dist
uses: casperdcl/deploy-pypi@v2
casperdcl marked this conversation as resolved.
Show resolved Hide resolved
with:
build: true
password: ${{ secrets.EQT_SECRET_TOKEN }}
upload: ${{ github.event_name == 'push' && startsWith(github.ref, 'refs/tags') }}
- if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags')
name: Release
run: |
changelog=$(git log --pretty='format:%d%n- %s%n%b---' $(git tag --sort=v:refname | tail -n2 | head -n1)..HEAD)
tag="${GITHUB_REF#refs/tags/}"
gh release create --title "Version $tag" --draft --notes "$changelog" "$tag" dist/${{ steps.dist.outputs.whl }} dist/${{ steps.dist.outputs.targz }}
Copy link
Member

Choose a reason for hiding this comment

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

Does this automatically create the version description from the git logs?
I think we would prefer to use the change log we manually write in CHANGELOG.md

Copy link
Member Author

Choose a reason for hiding this comment

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

yes but as per the updated .github/workflows/README.md instructions; it only creates a --draft release. Manual copyediting & approval of said draft is always required to make it public.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So the concept here is that we use git log to create the draft of the release text, taking from the commit messages. This text accompanies a GitHub draft release and will then be manually edited once we make the actual release.

I like the automation, however I feel that the release text is related to a GitHub feature rather than an integral part of the software.

Often I find myself editing the changelog differently than the commit messages. Maybe it is a good practice to write the commit messages better! My real argument against deleting the CHANGELOG.md is that we tend to cluster changes on topic rather than commit.

As I understand it, there will still be the chance to edit the draft text, but how can we find the final text not on GitHub?

Copy link
Member Author

@casperdcl casperdcl Jul 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I like the automation

It's not about automation. Automation is merely a happy bonus side-effect. It's about DRY/SSoT (see full explanation, including bugs I had to manually fix in the old release noted).

I feel that the release text is related to a GitHub feature rather than an integral part of the software.

  1. the GH release text was already being used before this PR
  2. dev metadata (version history) is NOT meant to be an "integral part of software" (that's the whole point of git commits as well as commit messages)
  3. best practice is to put release notes as comments in annotated tags or merge commits if you want them in a place where a dev will find them easily sans GH web interface
  4. release notes should link to relevant issues & PRs (for context & discussion), which is distinctly inherently a GH-only (non-local-codebase) use case.

Often I find myself editing the changelog differently than the commit messages. Maybe it is a good practice to write the commit messages better!

Still possible here. I do the same. The commit messages injected in the draft ensure:

  1. I don't forget to mention something important
  2. I'm embarrassed by poorly-written unfit-for-purpose messages (which I delete), inspiring me to write better in future

My real argument against deleting the CHANGELOG.md is that we tend to cluster changes on topic rather than commit.

  1. that's what merge commits & tags are for, ergo what should be described in merge commit messages & annotated tags
  2. the changelog was mirrored in two places, I'm simply proposing we delete one of those places

As I understand it, there will still be the chance to edit the draft text, but how can we find the final text not on GitHub?

Annotated tags & (merge) commit messages.

Finally, overall harsh truth:

  • high-level end users: only look at web material (PyPI, GH Readme, GH releases)
  • devs: only look at git commits & git blame
  • nobody: wants changelog file checked out in-repo

I really believe there is zero frustration if changelog.txt simply links to GH releases.

Copy link
Member Author

@casperdcl casperdcl Jul 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@paskino as discussed, I've now updated all git tags to have annotations containing the release notes.

You can git fetch --tags -f to get the updated annotations. To view a changelog without an internet connection1:

git config --global alias.changelog 'for-each-ref --sort=-*authordate --format="# %(contents:subject)%0a%(contents:body)" refs/tags'

git changelog

next step is to https://github.com/paskino/qt-elements/transfer to @TomographicImaging

Footnotes

  1. https://stackoverflow.com/questions/69765351/list-all-git-tags-with-their-annotations ↩

Copy link
Member Author

@casperdcl casperdcl Jul 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

added instructions & tests for annotated tags: 4e918e6

env:
GH_TOKEN: ${{ secrets.GH_TOKEN || github.token }}
10 changes: 9 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,9 @@
eqt/version.py
*.py[cod]
__pycache__/
/eqt/_dist_ver.py
/*.egg*/
/build/
/dist/
/.coverage*
/coverage.xml
/.pytest_cache/
66 changes: 0 additions & 66 deletions CHANGELOG.md
View file
Open in desktop

This file was deleted.

Loading