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

Add guide for writing a Provider? #2

Closed
alilleybrinker opened this issue Feb 6, 2024 · 4 comments
Closed

Add guide for writing a Provider? #2

alilleybrinker opened this issue Feb 6, 2024 · 4 comments

Comments

@alilleybrinker
Copy link

alilleybrinker commented Feb 6, 2024

It looks like new providers could be added in the following way:

  1. Define a provider type which implements the Provider trait (see GitHubReleaseProvider)
  2. Wire it up in the get_provider impl for DefaultProviderFactory in main.rs.

It would be nice to have this reflected in the documentation!

If y'all are open to contributions, I'd be happy to take a crack at it. Looks like the docs themselves are built with Docusaurus, which shouldn't be difficult to work with.

My only question to validate at the moment is the proper handling of the provider config. It looks like the intent is that the provider config is given as a valid "loosely parsed" JSON value, which the provider itself is then able to deserialize into any specific structure it likes. Do I have that right?

@alilleybrinker
Copy link
Author

(Disclosure: my interest here is that it would be nice to add a GitLab provider which works similarly to the GitHub provider, downloading released artifacts built by CI from a GitLab instance)

@bolinfest
Copy link
Contributor

From the current docs:

(At the time of this writing, there is no way to add custom providers without forking DotSlash.)

Relatedly, I don't think that we are ready to commit to the contract of Provider just yet. To be clear, I am open to considering #3, but it would have to be a built-in provider for now. That way, any changes to the Provider trait could be made atomically with their implementations.

My only question to validate at the moment is the proper handling of the provider config. It looks like the intent is that the provider config is given as a valid "loosely parsed" JSON value, which the provider itself is then able to deserialize into any specific structure it likes. Do I have that right?

Yes, that is correct.

@alilleybrinker
Copy link
Author

Totally makes sense regarding not wanting to commit to the Provider design at the moment. Perhaps then it doesn't make sense to document how to implement a Provider (the design today is also simple enough that a reasonably capable Rust programmer should in theory be able to figure out what's needed with relative ease). So long as the process of adding one stays the same, you could also always point people to this issue and to the draft PR for a GitLab Releases provider to show what's necessary.

There's also the fact that so far the docs for Dotslash appear to be focused on users of it, not contributors to it, so docs on how to write a Provider would be a little out of place there.

Thanks! Happy to consider this issue resolved for now then.

@bolinfest
Copy link
Contributor

@alilleybrinker FYI, I just put up an RFC for this: #7.

facebook-github-bot pushed a commit that referenced this issue Feb 18, 2025
Summary:
it's been a while since the last update of [rstest](https://docs.rs/rstest/latest/rstest/index.html) and and there's been many new features: https://github.com/la10736/rstest/releases

NOTE: It appears that the "crate-name" feature is not compatible with non-cargo builds as the one requiring a materialized Cargo.toml file ([source #1](https://github.com/la10736/rstest/blob/v0.24.0/rstest_macros/src/render/crate_resolver.rs#L9), [source #2](https://github.com/bkchr/proc-macro-crate/blob/master/src/lib.rs#L185-L186)). For that reason, I'm opting-out from that feature and enabling the rest of them ([list](https://docs.rs/crate/rstest/latest/features)), which is "async-timeout".

Reviewed By: dtolnay

Differential Revision: D69493566

fbshipit-source-id: 715cb1706b4f304f26b878250fe60708cc2142bd
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

No branches or pull requests

2 participants