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.

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

replace centos 7 with almalinux 8, add almalinux 9, centos stream 9, … #543

Merged
merged 3 commits into from
May 24, 2024

Conversation

jonathanspw
Copy link
Contributor

…fedora stable/rawhide

Fixes #527

Copy link

codecov bot commented May 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.13%. Comparing base (a0aebb6) to head (4216e23).
Report is 2 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #543      +/-   ##
============================================
- Coverage     70.17%   70.13%   -0.04%     
============================================
  Files           109      109              
  Lines         59904    59905       +1     
============================================
- Hits          42039    42016      -23     
- Misses        17865    17889      +24     

see 14 files with indirect coverage changes

…fedora stable/rawhide

Signed-off-by: Jonathan Wright <jonathan@almalinux.org>
Copy link
Member

@madolson madolson left a comment

Choose a reason for hiding this comment

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

Also kicked off another run just to double check they aren't flaky or anything. https://github.com/valkey-io/valkey/actions/runs/9226495711/job/25386381848

.github/workflows/daily.yml Outdated Show resolved Hide resolved
.github/workflows/daily.yml Outdated Show resolved Hide resolved
@@ -731,20 +774,20 @@ jobs:
echo "skiptests: ${{github.event.inputs.skiptests}}"
echo "test_args: ${{github.event.inputs.test_args}}"
echo "cluster_test_args: ${{github.event.inputs.cluster_test_args}}"
# On centos7 actions/checkout@v4 does not work, so we use v3
# ref. https://github.com/actions/checkout/issues/1487
Copy link
Member

Choose a reason for hiding this comment

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

:D Yay

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed :)

What about moving to more general tagging on the checkout action instead of using a straight commit hash?

Copy link
Member

Choose a reason for hiding this comment

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

I haven't read the issue, but apparently it was included in the openSSF score card.

.github/workflows/daily.yml Outdated Show resolved Hide resolved
@jonathanspw
Copy link
Contributor Author

Forked repositories have recurring workflows disabled by default so that shouldn't be needed.

image

@madolson
Copy link
Member

Forked repositories have recurring workflows disabled by default so that shouldn't be needed.

You aren't wrong. I'm not exactly sure of the history of that feature, but we didn't want to change it now because it might have wider implications for folks that pull on their forks. Maybe now is the time to change it.

@madolson
Copy link
Member

@jonathanspw Can you fix the DCO check (sign-off on the last three commits). Otherwise I can do it since you were taking my changes.

@jonathanspw
Copy link
Contributor Author

@jonathanspw Can you fix the DCO check (sign-off on the last three commits). Otherwise I can do it since you were taking my changes.

I used the GH GUI to pull those in. No clue how to do DCO with the GUI. Can you do it?

@madolson
Copy link
Member

madolson commented May 24, 2024

@jonathanspw I'm sorry, I pushed the wrong branch to your unstable branch and github did not appreciate that (and closed the PR, so I can't update it anymore). Would you mind pushing the content https://github.com/valkey-io/valkey/tree/unstable_jon onto your unstable branch? That should pass all of the checks and will allow my to re-open and merge the PR.

Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Jonathan Wright <jonathan@almalinux.org>
@jonathanspw
Copy link
Contributor Author

I did a forced push to my branch that included that. Will it let you reopen it now?

@madolson madolson reopened this May 24, 2024
@jonathanspw
Copy link
Contributor Author

Hmm I didn't notice there's a piece in normal CI that does a build on CentOS 7. If you'll give me a minute I can add a commit that will change that as well, unless you'd rather it come in a separate PR.

@madolson
Copy link
Member

madolson commented May 24, 2024

Hmm I didn't notice there's a piece in normal CI that does a build on CentOS 7. If you'll give me a minute I can add a commit that will change that as well, unless you'd rather it come in a separate PR.

Up to you, I don't have a strong preference. Although I guess it makes sense to do in this PR.

Signed-off-by: Jonathan Wright <jonathan@almalinux.org>
@jonathanspw
Copy link
Contributor Author

This is ready to merge. Example of a run with the new code in place: https://github.com/jonathanspw/valkey/actions/runs/9228120347/job/25391510821

@madolson madolson merged commit 1c55f3c into valkey-io:unstable May 24, 2024
17 checks passed
PingXie pushed a commit to PingXie/valkey that referenced this pull request Jul 10, 2024
replace centos 7 with almalinux 8, add almalinux 9, centos stream 9, fedora stable, rawhide

Fixes valkey-io#527

---------

Signed-off-by: Jonathan Wright <jonathan@almalinux.org>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Ping Xie <pingxie@google.com>
PingXie pushed a commit to PingXie/valkey that referenced this pull request Jul 12, 2024
replace centos 7 with almalinux 8, add almalinux 9, centos stream 9, fedora stable, rawhide

Fixes valkey-io#527

---------

Signed-off-by: Jonathan Wright <jonathan@almalinux.org>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Ping Xie <pingxie@google.com>
PingXie pushed a commit that referenced this pull request Jul 12, 2024
replace centos 7 with almalinux 8, add almalinux 9, centos stream 9, fedora stable, rawhide

Fixes #527

---------

Signed-off-by: Jonathan Wright <jonathan@almalinux.org>
Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Signed-off-by: Ping Xie <pingxie@google.com>
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.

Replace CentOS 7 image with CentOS Stream 9
3 participants