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

API Design? #2

Open
epage opened this issue Apr 5, 2018 · 5 comments
Open

API Design? #2

epage opened this issue Apr 5, 2018 · 5 comments
Labels
breaking-change question Further information is requested

Comments

@epage
Copy link
Contributor

epage commented Apr 5, 2018

Use cases

  • A future version of cargo-when
  • stager: in a tool for laying out files for packaging, I want some parts of ci-detective available for variable substitutions
  • cargo-make uses ci_info to not auto-update if within CI
  • For testing this crate, it'd be ideal to keep it easy to dump all of the data

Design Requirements

From that, the needs are:

  • Query a select few attributes (like branch name) across CI systems
  • Dump all data

Inputs

  • serde is a push system, we don't know what fields are requested, so we'd have to load them all
  • std::env always allocates because (1) of locks and (2) of Windows string types

Some key points about the existing API

  • Every attribute is queried and allocated for a given CI system
  • There isn't an easy way to ask a question regardless of CI system

Proposal

  • ci_detective::<CI>::<CI> has functions to query and return the value if possible
  • ci_detective::is_ci() will do a fast-path check if running in a CI
  • ci_detective::CI::new() will be Option<CI>
    • CI will have methods for common CI queries. Closed-form polymorphism will be used to make it easier to change without being a breaking change.

Open Questions

  • Should we encourage query names to be consistent across CI systems?
    • Easier to break API when new information comes out
    • Easier for user to find what they want
  • Should the structs implement Deserializer to support converting to Deserialize types?
    • One way to allow dumping all data
    • While all data has to be parsed, it allows people to opt-in to only what they want
    • Probably requires aligning query names so people can make cross-CI queries

Alternatives

Load all data into structs

  • Slow loading and parsing all that data
  • Probably not in a format people would not anyways

Free functions

Structs that offer both lazy and eager loading

See #3

  • Requires any lazy load methods to be &mut
@epage epage added question Further information is requested breaking-change labels Apr 5, 2018
@epage

This comment has been minimized.

@epage

This comment has been minimized.

@epage

This comment has been minimized.

@epage
Copy link
Contributor Author

epage commented Jan 8, 2019

@CAD97, I'm trying to make sure we have an idea of all expected use cases. Do you have any you'd like to add that helped motivate you to write this?

@CAD97
Copy link
Collaborator

CAD97 commented Jan 9, 2019

I've actually not used it (😆), I just had an idea I really wanted to try out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants