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

Remove procfs dependency #643

Merged
merged 5 commits into from
Jul 12, 2023
Merged

Remove procfs dependency #643

merged 5 commits into from
Jul 12, 2023

Conversation

tamird
Copy link
Member

@tamird tamird commented Jul 11, 2023

  • Remove verifier log special case
  • Remove procfs dependency
  • Rewrite kernel version logic

This restores and enhances the logic originally added in #579.

@netlify
Copy link

netlify bot commented Jul 11, 2023

Deploy Preview for aya-rs-docs ready!

Name Link
🔨 Latest commit a1e0130
🔍 Latest deploy log https://app.netlify.com/sites/aya-rs-docs/deploys/64ae2388a35d3100084e3011
😎 Deploy Preview https://deploy-preview-643--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.

aya/src/util.rs Outdated
}

/// Returns the kernel version of the currently running kernel.
pub fn current() -> Result<Self, String> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should return a Box<dyn Error> instead of String here

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you help me understand how that's better? Currently aya does not depend on anyhow, so other than using strings I can't propagate context.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is now a public API, and returning String as Err is frowned upon (i think there's even a clippy lint for it)

Currently aya does not depend on anyhow, so other than using strings I can't propagate context.

libraries should depend on thiserror, apps on anyhow. You can define a thiserror derived error, give it one variant for each string you're returning now, and type erase it when you return it. The result is the same as using a string (Error implements Display), but it's not a String, gives you a backtrace on nightly if you want one etc

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

This restores and enhances the logic originally added in #579.
Incorrectly inverted in b611038.
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.

2 participants