Taking the spurs crate to 1.0

announcements
1

Hi all,

I'm thinking of giving spurs 1.0 status. I've been using this API for a while now for my own day-to-day work, and I think it strikes a reasonable balance between flexibility and ergonomics.

You can find the docs here: spurs - Rust

Before I do take it to 1.0, I was wondering what others think. Any feedback is welcome, but smaller suggestions are more likely to actually get implemented. PRs are also welcome.

Thanks!

2

I realize that this isn't a small suggestion, but go through the old libz blitz API guideline checklist. You should be able to click the checkmark on here to mark them off when you complete them (I've already started going through some of them to see what you've already implemented).

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
    • 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)
    • 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)
    • [*] Input syntax is evocative of the output (C-EVOCATIVE)
    • [*] Macros compose well with attributes (C-MACRO-ATTR)
    • [*] Item macros work anywhere that items are allowed (C-ANYWHERE)
    • [*] Item macros support visibility specifiers (C-MACRO-VIS)
    • [*] Type fragments are flexible (C-MACRO-TY)
  • Documentation (crate is abundantly documented)
    • Crate level docs are thorough and include examples (C-CRATE-DOC)
    • All items have a rustdoc example (C-EXAMPLE)
    • [*] Examples use ?, not try!, not unwrap (C-QUESTION-MARK)
    • Function docs include error, panic, and safety considerations (C-FAILURE)
    • Prose contains hyperlinks to relevant things (C-LINK)
    • [*] Cargo.toml includes all common metadata (C-METADATA)
      • authors, description, license, homepage, documentation, repository,
        readme, keywords, categories
    • [*] Crate sets html_root_url attribute "https://docs.rs/CRATE/X.Y.Z" (C-HTML-ROOT)
    • [*] Release notes document all significant changes (C-RELNOTES)
    • [*] Rustdoc does not show unhelpful implementation details (C-HIDDEN)
  • 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)
  • 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)
7 Likes
3

  • [*] toggle_dry_run is too error-prone and state-specific. Just use set_dry_run(bool).

  • [*] Please remove the homepage entry, since it is identical to repository. As described in C-METADATA:

    The homepage metadata should only be set if there is a unique website for the crate other than the source repository or API documentation. Do not make homepage redundant with either the documentation or repository values. For example, serde sets homepage to https://serde.rs , a dedicated website.

  • [*] SshShell and SshSpawnHandle do not implement Debug.

  • [*] This crate is licensed under the ASLv2 (Apache-2.0) Unless you have a specific reason for doing so, please use the Rust dual-license MIT OR Apache-2.0. Among other reasons, the ASLv2 is not compatible with the GPLv2, while the MIT license is.

4

Crate and its dependencies have a permissive license (C-PERMISSIVE)

I was a bit shocked to read this statement. Should I interpret it as:

The GPL is not welcome here.

or

Most crates in the Rust ecosystem are dual licenced MIT/Apache and if you want a crate to be widely used and compatible, then use a permissive licence.

If its the second interpretation, I would recommend maybe softening the language a little bit -- I went with the first interpretation and it took me a while to calm down.

4 Likes
5

The commit removing html_root_url was reverted and this note was added to the guidelines, so this still needs to be done.

1 Like
6

You're probably interested in https://github.com/rust-lang-nursery/api-guidelines/issues/122#issuecomment-370964876 then.

7

Thanks @notriddle! I had forgotten about that checklist. I will go through it (albeit slowly). If you have time/interest, an objective reviewer would be welcome.

1 Like
8

Btw, I am still working on this very slowly. As I have been using the library for my work a few bugs got fixed, and some modest interface improvements have been made. I suspect that many of the items in the checklist already hold, but I need to actually confirm them...