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

handle unwrap while getting current kernel version #1042

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pythops
Copy link

@pythops pythops commented Sep 27, 2024

Closes #1024

There is still 2 places where we still unwrap though. To handle it there might require changes to some functions that I am not sure if they should be part of this PR


This change is Reviewable

Copy link

netlify bot commented Sep 27, 2024

Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit b2f8742
🔍 Latest deploy log https://app.netlify.com/sites/aya-rs-docs/deploys/66f7fdd67c4397000885f804
😎 Deploy Preview https://deploy-preview-1042--aya-rs-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mergify mergify bot added the aya This is about aya (userspace) label Sep 27, 2024
@tamird
Copy link
Member

tamird commented Sep 28, 2024

I won't speak for @alessandrod but IMO logging errors is not what we want to do here. We should instead propagate those errors, probably.

@alessandrod
Copy link
Collaborator

I won't speak for @alessandrod but IMO logging errors is not what we want to do here. We should instead propagate those errors, probably.

IMO it's not worth polluting the API for a condition that should effectively never happen. At some point, we'll fix our errors so that they don't have 2000 variants and users can do exhaustive matches. Doing an exhaustive match on "the kernel version can't be detected" is unnecessary cognitive overhead. We should always be able to detect the kernel version and we are. There are some obscure edge cases with distros very few people use (proxmox) and in that case I think it's sufficient to warn and execute the most conservative code. Warnings are shown by default, so they're unlikely to go undetected.

@pythops
Copy link
Author

pythops commented Sep 28, 2024

Any review guys 🙏

@tamird
Copy link
Member

tamird commented Nov 22, 2024

This has merge conflicts.

Copy link

mergify bot commented Nov 24, 2024

@pythops, this pull request is now in conflict and requires a rebase.

@mergify mergify bot added the needs-rebase label Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aya This is about aya (userspace) needs-rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove instances of KernelVersion::current().unwrap()
3 participants