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

New reorganized modular libraries #26

Open
MingweiSamuel opened this issue Nov 7, 2019 · 5 comments
Labels
Milestone

Comments

@MingweiSamuel
Copy link
Owner

@MingweiSamuel MingweiSamuel commented Nov 7, 2019

We want to support additional APIs: static data (cdragon & ddragon), LCU, and maybe some others in the future. We also want to keep things nice and modular so we don't end up with one mega package.

I'm thinking the following layout (For package names/namespaces)

  • MingweiSamuel.Camille - "meta package" of all the other packages

  • Camille.Enums - Enums in a separate package to be shared by other modules.

  • Camille.RiotApi - current .api.riotgames.com requester & classes.

    • Could go for even smaller granularity and separate lol, tft, lol-tournament etc. Might be good idea considering Riot's upcoming releases.
  • Camille.LCU #24

  • Camille.DataDragon #16

  • MingweiSamuel.AsyncTokenBucket - not part of "Camille," just pull the token bucket into a separate package for general use.

Possible additions if they're needed:

  • Camille.CommunityDragon
  • (Camille.LorDataDragon)
  • (Camille.Esports)

Any thoughts? @RyadaProductions

@MingweiSamuel MingweiSamuel added this to the 3.0.0 milestone Nov 7, 2019
@RyadaProductions

This comment has been minimized.

Copy link
Contributor

@RyadaProductions RyadaProductions commented Nov 7, 2019

Regarding the idea of splitting it up in more modular/granular packages i am highly supportive.
Though I personally would feel that it would be the best to cleanup the namespace as well. And with that i mean, removing the MingweiSamuel part. I do understand this is your library, but having namespaces follow folder structure is a lot easier to maintain/use.
And the standard is for the namespace to start on the same project-name. (Since we are modernizing the library anyway i feel like this would be a good step forward).
But if your opinion is that you want to keep that in the namespace, then I am fine with that. It just feels like useless clutter.

@MingweiSamuel

This comment has been minimized.

Copy link
Owner Author

@MingweiSamuel MingweiSamuel commented Nov 7, 2019

I don't have any objections to removing the MingweiSamuel., though I feel like it would be missing the org part of the convention

MingweiSamuel added a commit that referenced this issue Nov 9, 2019
@RyadaProductions

This comment has been minimized.

Copy link
Contributor

@RyadaProductions RyadaProductions commented Nov 11, 2019

Yeah i forgot that the Organisation part was there. Although I do think that one is optional since the System namespace doesn't use any organization as well as almost all other libraries that I encountered and worked with. It would just be a question of how closely do we want to follow the spec.

@MingweiSamuel

This comment has been minimized.

Copy link
Owner Author

@MingweiSamuel MingweiSamuel commented Nov 11, 2019

I removed the MingweiSamuel and updated the top post

@MingweiSamuel

This comment has been minimized.

Copy link
Owner Author

@MingweiSamuel MingweiSamuel commented Nov 12, 2019

I found HttpClient has default host/port/headers which has let me pull out some really nice abstraction layers. So I'm thinking the following hierarchy now:

  • Camille.Enums
    • Camille.RiotApi(.Base)
      • Camille.RiotApi.Lol
      • Camille.RiotApi.Tft
      • Camille.RiotApi.LolTournament
    • Camille.Lcu(.Base)
      • ???
      • ???
  • Camille.DDragon
  • etc

Could go as granular as a package for each set of endpoints .. though that would probably be excessive (though it might be good for micro-optimizing app sizes)

Also should have a version of Camille.Lcu ready for testing tonight, hopefully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.