8000 Consistent number of arguments across OSes · Issue #29 · kstrafe/file-rotate · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Consistent number of arguments across OSes #29

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

Closed
proski opened this issue Feb 14, 2025 · 4 comments · Fixed by #30
Closed

Consistent number of arguments across OSes #29

proski opened this issue Feb 14, 2025 · 4 comments · Fixed by #30

Comments

@proski
Copy link
proski commented Feb 14, 2025

Would it be possible to make FileRotate::new() accept the same number of arguments on all platforms? The current implementation is hurting portability. My project depends on a proprietary crate that's always using 5 arguments, and that causes a compile error. Dozens of dependencies compile just fine, file-rotate is the only one that breaks compilation. Sure, the other crate is wrong, but file-rotate makes it easy to make that mistake and hard to catch it (testing on multiple platforms is needed).

I realize that the change break compatibility, so let's make a choice we won't have to regret.

Possible approaches:

  • take a structure (not enum) that defines permissions for all OSes we care about
  • use the builder pattern with the same effect (with_posix_perms, with_windows_perms), deprecate new
  • use some abstraction that maps to POSIX and Windows permissions - that's probably too restrictive
@Ploppz
Copy link
Collaborator
Ploppz commented Feb 25, 2025

Good point. I'll take a look this week to see how it will look. Not sure yet which solution is best.

@Ploppz
Copy link
Collaborator
Ploppz commented Feb 27, 2025

@proski How does this solution work for your use case?

@proski
Copy link
Author
proski commented Feb 27, 2025

That would work for my case.
Thank you!

@Ploppz
Copy link
Collaborator
Ploppz commented Feb 27, 2025

Published version 0.8.0 to crates.io :)

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 a pull request may close this issue.

2 participants
0