Skip to content

Exclude files specifically changed for release from rc sync PR #1643

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mehrdad2m
Copy link
Contributor

Context:
During feature freeze, we need to modify some of the Github workflows on the release candidate branch. These changes should not be included in the daily sync that merged into main branch.

Description of the Change:

  • Added the check-catalyst workflow which should be modified in feature freeze week to be excluded.
  • Added files that are modified in the "Bump PennyLane and Lightning minimum versions" PR that is merged to rc on release day.

Benefits:
Less maintenance of the daily rc sync PR and therefore less risk for mistakenly merging unwanted code

Possible Drawbacks:
We would require a version bump PR on main after the release to bump the versions of Pennylane and Lightning.
Any change to the wheel builder scripts that actually need to be included in main should be applied separately but not sure if that is something that happens frequently.

Related GitHub Issues:

@mehrdad2m
Copy link
Contributor Author

mehrdad2m commented Apr 11, 2025

@dime10 , My original motivation was to only exclude the check-catalyst.yaml file, but not too sure about the rest. I am not sure how frequently we would have changes in the other files that want in the main too. Do you think it make sense to exclude them?

@mehrdad2m mehrdad2m requested a review from a team April 11, 2025 17:08
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md on your branch with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

Copy link

codecov bot commented Apr 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.87%. Comparing base (0d9585b) to head (6123f18).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1643   +/-   ##
=======================================
  Coverage   96.87%   96.87%           
=======================================
  Files          80       80           
  Lines        8831     8845   +14     
  Branches      837      839    +2     
=======================================
+ Hits         8555     8569   +14     
  Misses        222      222           
  Partials       54       54           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@paul0403
Copy link
Member

paul0403 commented May 5, 2025

Is this still relavant?

Comment on lines +70 to +73
git checkout origin/main -- .github/workflows/scripts/linux_arm64/rh8/test_wheels.sh
git checkout origin/main -- Makefile
git checkout origin/main -- doc/requirements.txt
git checkout origin/main -- setup.py
Copy link
Contributor

@dime10 dime10 May 5, 2025

Choose a reason for hiding this comment

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

@dime10 , My original motivation was to only exclude the check-catalyst.yaml file, but not too sure about the rest. I am not sure how frequently we would have changes in the other files that want in the main too. Do you think it make sense to exclude them?

Thanks Mehrdad, yeah good question, this does look like a lot of files 🤔 I think all the actions are probably fine, since we do inject those patches there during the release process. The .dep-versions and doc/requirements.txt are also updated according to the release guide. For the Makefile and setup.py I'm not sure, we could leave those out for now and see later? The linux scripts are gone now (🎉), so that one can be removed :)

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.

3 participants