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

Follow the Rust API Guidelines #144

Closed
54 tasks done
madsmtm opened this issue Aug 31, 2023 · 2 comments
Closed
54 tasks done

Follow the Rust API Guidelines #144

madsmtm opened this issue Aug 31, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@madsmtm
Copy link
Member

madsmtm commented Aug 31, 2023

Let's ensure that the library follows the official API guidelines (although I suspect it does on almost all accounts). I've posted the checklist below, and will try to fill it out as I see fit (opening issues for non-resolved items).

Rust API Guidelines Checklist

  • Naming (crate aligns with Rust naming conventions)
    • Casing conforms to RFC 430 (C-CASE)
    • Ad-hoc conversions follow as_, to_, into_ conventions (C-CONV)
    • Getter names follow Rust convention (C-GETTER)
    • Methods on collections that produce iterators follow iter, iter_mut, into_iter (C-ITER)
    • Iterator type names match the methods that produce them (C-ITER-TY)
    • Feature names are free of placeholder words (C-FEATURE)
    • Names use a consistent word order (C-WORD-ORDER)
  • Interoperability (crate interacts nicely with other library functionality)
    • Types eagerly implement common traits (C-COMMON-TRAITS)
      • Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug,
        Display, Default
      • Note: For Copy, see Does RawWindowHandle need to be Copy? #138.
      • Note: Ordering traits would perhaps be ill-advised, as most of these are pointers, and the actual ordering would be rather unexpected?
      • Note: Display is probably undesired, since we don't pull in the libraries to give a proper description of the handles.
    • Conversions use the standard traits From, AsRef, AsMut (C-CONV-TRAITS)
    • Collections implement FromIterator and Extend (C-COLLECT)
    • Data structures implement Serde's Serialize, Deserialize (C-SERDE)
      • Note: Window handles cannot and should not be serialized or deserialized, especially not from disk; they are ephemeral, and very much tied to the lifetime of the program!
    • Types are Send and Sync where possible (C-SEND-SYNC)
    • Error types are meaningful and well-behaved (C-GOOD-ERR)
    • Binary number types provide Hex, Octal, Binary formatting (C-NUM-FMT)
    • Generic reader/writer functions take R: Read and W: Write by value (C-RW-VALUE)
  • Macros (crate presents well-behaved macros)
  • Documentation (crate is abundantly documented)
  • Predictability (crate enables legible code that acts how it looks)
    • Smart pointers do not add inherent methods (C-SMART-PTR)
    • Conversions live on the most specific type involved (C-CONV-SPECIFIC)
    • Functions with a clear receiver are methods (C-METHOD)
    • Functions do not take out-parameters (C-NO-OUT)
    • Operator overloads are unsurprising (C-OVERLOAD)
    • Only smart pointers implement Deref and DerefMut (C-DEREF)
    • Constructors are static, inherent methods (C-CTOR)
  • Flexibility (crate supports diverse real-world use cases)
    • Functions expose intermediate results to avoid duplicate work (C-INTERMEDIATE)
    • Caller decides where to copy and place data (C-CALLER-CONTROL)
    • Functions minimize assumptions about parameters by using generics (C-GENERIC)
    • Traits are object-safe if they may be useful as a trait object (C-OBJECT)
  • Type safety (crate leverages the type system effectively)
    • Newtypes provide static distinctions (C-NEWTYPE)
    • Arguments convey meaning through types, not bool or Option (C-CUSTOM-TYPE)
    • Types for a set of flags are bitflags, not enums (C-BITFLAG)
    • Builders enable construction of complex values (C-BUILDER)
  • Dependability (crate is unlikely to do the wrong thing)
    • Functions validate their arguments (C-VALIDATE)
      • Note: It's usually impossible to check if a pointer is valid, so we just have to assume it is; that's why it's unsafe!
      • Note: We could verify the integer handles, but that would introduce dependencies into this crate, while avoiding those is part of the reason to have it in the first place.
    • Destructors never fail (C-DTOR-FAIL)
    • Destructors that may block have alternatives (C-DTOR-BLOCK)
  • Debuggability (crate is conducive to easy debugging)
  • Future proofing (crate is free to improve without breaking users' code)
  • Necessities (to whom they matter, they really matter)
    • Public dependencies of a stable crate are stable (C-STABLE)
    • Crate and its dependencies have a permissive license (C-PERMISSIVE)
@madsmtm madsmtm added the enhancement New feature or request label Aug 31, 2023
@madsmtm madsmtm mentioned this issue Aug 31, 2023
4 tasks
@Lokathor
Copy link
Contributor

There's only two unchecked boxes:

  • We can't have private fields, that's explicitly against the point of this crate.
  • We don't have any public dependencies.

So those boxes should be checked, and then this issue closed? Or did i miss something else?

@madsmtm
Copy link
Member Author

madsmtm commented Sep 1, 2023

Heh, I've been working on this issue for the past hour, that's why there's only two missing. But I guess I'll just post my thoughts here, and be done with it.

Re private fields:
It might actually make sense for e.g. the error enum, e.g. add a pub struct NotSupported(());, and then require that in HandleError::NotSupported(NotSupported::default()), in case we wanted the ability to add more information later.

But I suppose that the error type is #[non_exhaustive], so we could add more information that way, and besides, we should rather do #143 if we're concerned about extensibility in the error type.

Re public deps:
This was being considered in #134, so I wanted to ensure that we didn't release v1.0 before the dependency did that; but then I read it again, and realised that @notgull already considered that, so I don't think I need to worry about it.

@madsmtm madsmtm closed this as completed Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants