-
Notifications
You must be signed in to change notification settings - Fork 39
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
Merge v0.19.0 release fixes onto master #170
Conversation
* Added auto version bump functionality to pl lightning (#159) * Added auto version bump functionality to pl lightning * Update .github/workflows/pre_release_version_bump.yml removed trailing white space * Update .github/workflows/post_release_version_bump.yml Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca> * Update .github/workflows/pre_release_version_bump.yml Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca> * removed flag for pennylane release as it is not required Co-authored-by: antalszava <antalszava@gmail.com> * Update pre_release_version_bump.yml (#160) * Update pre_release_version_bump.yml * Update pre_release_version_bump.yml * Update pre_release_version_bump.yml * Update .github/workflows/pre_release_version_bump.yml * Apply suggestions from code review * Update post_release_version_bump.yml swap main --> master * Update CHANGELOG.md To be consistent with other plugins and not cause errors for the auto version bumping * pre release version bump * updates * Update pennylane_lightning/_version.py * formatting updates to changelog Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca> Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: antalszava <antalszava@users.noreply.github.com>
* Fix OMP issue on M1 macs * Update changelog
* Fix OMP issue on M1 macs * Update changelog * Fix CI builder
* Fix OMP with ARM MacOS * Ensure wheels uploaded to GH * Separate out noarch wheels
Codecov Report
@@ Coverage Diff @@
## master #170 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 182 182
=========================================
Hits 182 182
Continue to review full report at Codecov.
|
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.
Thanks @mlxd!
@@ -55,7 +57,7 @@ jobs: | |||
CIBW_ARCHS_LINUX: ${{matrix.arch}} | |||
|
|||
- uses: actions/upload-artifact@v2 | |||
if: github.ref == 'refs/heads/master' | |||
if: ${{ github.event_name == 'release' || github.ref == 'refs/heads/master' }} |
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 the or
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.
Good question. I think ||
is supported (link https://docs.github.com/en/actions/learn-github-actions/expressions#operators) though happy to modify if you think or
is better.
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.
Ah, I misunderstood. Yea, we want this to run in both cases. We can issue a release from a non master branch (the tagged v0.xy branch), and ensure the wheels are built. Master may have since move on, so we need to be able to support both ( I think).
on: | ||
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.
Do we need the "types" line, as we do e.g. for wheel_linux_ppc64le.yml
? (same question applies to some of the other workflows)
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.
Good catch. I think these need to be added to all.
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.
Good catch -- I think these need to be added to all workflows.
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.
Rather than changing the RC, maybe we merge this as is and modify in a subsequent PR. This won't change the wheel, and help with consistency across the wheel history. What do you think?
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.
Sounds good! Thanks @mlxd!
Context: Bugfixes from v0.19.0 release
Description of the Change:
Benefits:
Possible Drawbacks:
Related GitHub Issues: