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

chore: rebase the latest release branch on push to main #24

Merged
merged 1 commit into from
Oct 9, 2024
Merged

chore: rebase the latest release branch on push to main #24

merged 1 commit into from
Oct 9, 2024

Conversation

nsarlin-zama
Copy link
Collaborator

@nsarlin-zama nsarlin-zama commented Oct 8, 2024 •
edited
Loading

IceTDrinker
IceTDrinker reviewed Oct 8, 2024
View reviewed changes
Copy link
Member

@IceTDrinker IceTDrinker left a comment

Choose a reason for hiding this comment

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

Can you explain what the workflow is here please ?

@nsarlin-zama
Copy link
Collaborator Author

Yes, it runs on pushes to the main branch and rebase the latest version branch (e.g. "v0.2") to it:

  • git for-each-ref --format='%(refname:short)' refs/remotes/origin: list the branches
  • grep -Po '(?<=origin/)v\d.\d(.\d)?$': filter the "version" branches: vX.Y(.Z)
  • sort -V | tail -n : sort based on the version number and takes the highest one

@IceTDrinker
Copy link
Member

Yes, it runs on pushes to the main branch and rebase the latest version branch (e.g. "v0.2") to it:

I don't recall all our discussions around this, it's a way to keep the latest version in sync with main right ? but still having versions to avoid relying on pulling main in TFHE-rs ?

IceTDrinker
IceTDrinker reviewed Oct 8, 2024
View reviewed changes
.github/workflows/rebase_latest_version.yml
run: |
git switch $DATA_LATEST_VERSION
git rebase main
git push --force-with-lease
Copy link
Member

Choose a reason for hiding this comment

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

if this fails we likely need a notification in our updates channel wdyt ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can do that but I don't think this repo has acces to the slack bot token

Copy link
Member

Choose a reason for hiding this comment

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

see with @soonum then please

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The token has been enabled for this repo, so now it should work

@nsarlin-zama
Copy link
Collaborator Author

nsarlin-zama commented Oct 8, 2024 •
edited
Loading

Yes, it runs on pushes to the main branch and rebase the latest version branch (e.g. "v0.2") to it:

I don't recall all our discussions around this, it's a way to keep the latest version in sync with main right ? but still having versions to avoid relying on pulling main in TFHE-rs ?

Yes that's the idea.
This repo is itself versioned. The goal is to be able to run data tests on previous branches of tfhe-rs. For example tfhe-rs 0.7 depends on the version 0.1 of this repo whereas tfhe-rs 0.8 and main use data v0.2.
Adding new data types does not require to upgrade this version number, but modifying the metadata of previous test types does.

@nsarlin-zama nsarlin-zama force-pushed the ns/auto-rebase_latest_branch branch from 0083c97 to 571a480 Compare October 8, 2024 12:37
IceTDrinker
IceTDrinker reviewed Oct 8, 2024
View reviewed changes
.github/workflows/rebase_latest_version.yml Outdated
Comment on lines 31 to 37
- name: Slack Notification
if: ${{ always() && job.status == 'failure' }}
continue-on-error: true
uses: rtCamp/action-slack-notify@4e5fb42d249be6a45a298f3c9543b111b02f7907
env:
SLACK_COLOR: ${{ job.status }}
SLACK_MESSAGE: "backward-compat-data version auto-rebase failed: (${{ env.ACTION_RUN_URL }})"
Copy link
Member

Choose a reason for hiding this comment

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

could we have the PR number or even a link embedded in the notification such that we can take action more quickly in case of failure ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's what ${{ env.ACTION_RUN_URL }} is for ?

I copied the slack part from tfhe-rs, the same pattern is used in most of our workflows

Copy link
Member

Choose a reason for hiding this comment

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

ah wait, no I'm mistaken, we would need the branch that failed to rebase, not a PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok I updated it to show the name of the branch

@nsarlin-zama nsarlin-zama force-pushed the ns/auto-rebase_latest_branch branch from 571a480 to 2466885 Compare October 8, 2024 15:41
IceTDrinker
IceTDrinker approved these changes Oct 8, 2024
View reviewed changes
Copy link
Member

@IceTDrinker IceTDrinker left a comment

Choose a reason for hiding this comment

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

Thanks a lot ! Sorry for the tedious review

@nsarlin-zama nsarlin-zama merged commit 2dd8507 into main Oct 9, 2024
1 check passed
@nsarlin-zama nsarlin-zama deleted the ns/auto-rebase_latest_branch branch October 9, 2024 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IceTDrinker IceTDrinker IceTDrinker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nsarlin-zama @IceTDrinker