-
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
feat: gated fsync kernel (matches coreos-stable) #234
Conversation
.github/workflows/reusable-build.yml
Outdated
@@ -77,7 +80,11 @@ jobs: | |||
export BUILDER_IMAGE=quay.io/fedora/fedora | |||
echo "BUILDER_IMAGE=${BUILDER_IMAGE}" >> $GITHUB_ENV | |||
echo "FQ_BUILDER_IMAGE=${BUILDER_IMAGE}:${{ matrix.fedora_version }}" >> $GITHUB_ENV | |||
export KERNEL_IMAGE=${{ matrix.kernel_flavor }}-kernel | |||
if [[ "${{matrix.kernel_flavor}}" != "fsync-gated" ]]; then |
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.
A little nitpicky, but why is this a negative? Isn't it a bit more clear to:
if [[ "${{matrix.kernel_flavor}}" == "fsync-gated" ]]; then
export KERNEL_IMAGE=fsync-kernel
else
export KERNEL_IMAGE=${{ matrix.kernel_flavor }}-kernel
fi
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.
Only reason was that's just how I was thinking. I agree that not using the negative 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.
My only comment is I struggle with the name 'fsync-gated' ... it does signify a delayed kernel release, but it doesn't connect much to the fact that we're matching the coreos-stable kernel version.
The thoughts I've had seem either too wordy or suffer from the same problem but just disconnecting another concept.
fsync-coreos-stable
fsync-coreos
fsync-stable
???
Thoughts on another name, or just leave it as is.
@m2Giles there was a comment made in Discord about the zfs 2.2.6 is pending release and will address 6.10 builds. openzfs/zfs#16472 |
For name I think fsync-coreos should get the idea across. fsync-coreos-stable would be clearer but unsure if we need that distinction |
The one failing build now is just for coreos-testing-zfs, which will fail until openzfs releases version 2.2.6. This failure should not block merging of this PR. |
Thank you for contributing to the Universal Blue project!
Please read the Contributor's Guide before submitting a pull request.