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

V2 bringup #3

Closed
wants to merge 4 commits into from
Closed

V2 bringup #3

wants to merge 4 commits into from

Conversation

4JX
Copy link
Contributor

@4JX 4JX commented Feb 1, 2022

This is still a WIP, but I might as well open it already. V1 to V2 migration guide can be found here

  • Routes have been corrected to account for the new naming (mod > project).
  • Checked for feature parity with the API repo and added/renamed accordingly

To consider:

  • The wording of comments, methods and structs is most likely going to need to be changed as mods are now called projects in preparation for modpack support
  • permissions in TeamMember is defined here
  • Using #[serde(deny_unknown_fields)] to catch inconsistencies earlier may be useful
  • Re-export Bytes to avoid having to explicitly add it to the dependency list if not used anywhere else.

I'd want to see support added for using filters in the queries. Maybe something like the following could work to still keep the text based ones simple:

struct Query {}

impl From<String> for Query {}

And then some logic inside each function (get_mod, list_mods, etc) to handle the filters that are valid for the endpoint being reached.

@4JX 4JX marked this pull request as draft February 1, 2022 23:21
@theRookieCoder
Copy link
Collaborator

theRookieCoder commented Feb 2, 2022

Amazing! I will implement the search mods api soon (I'm busy with Ferium and related projects rn). Also it seems that only listing project's versions has query filters?

@4JX
Copy link
Contributor Author

4JX commented Feb 2, 2022

Amazing! I will implement the search mods api soon (I'm busy with Ferium and related projects rn). Also it seems that only listing project's versions has query filters?

The idea was to unify how you add filters/parameters for searching + version list + the others that require auth (should those eventually ne added) under a common type, though on a second thought it will lead to a false sense of "oh this is a valid parameter for this query".

@4JX
Copy link
Contributor Author

4JX commented Feb 2, 2022

Did an attempt on adding filtering to list_versions, lmk what you think.
4JX@69d23d0

Noticed a typo, that'd be fixed

@theRookieCoder
Copy link
Collaborator

Very cool! Although using string manipulation at this point would be quite dangerous. I think we should use url or something like that

@theRookieCoder theRookieCoder marked this pull request as ready for review February 3, 2022 07:03
@theRookieCoder
Copy link
Collaborator

theRookieCoder commented Feb 3, 2022

Hey! I've added this PR to the v2-bringup branch on the main repo. You're a collaborator now so we can work together on that branch. We'll PR from that branch here. (am I doing this right?)

@theRookieCoder theRookieCoder mentioned this pull request Feb 3, 2022
@4JX 4JX deleted the v2-bringup branch February 4, 2022 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants