superfaceai / service-client

Library provides client for superface backend apis.
MIT License
5 stars 1 forks source link

feat: Listing profiles, maps & providers #53

Closed kysely closed 3 years ago

kysely commented 3 years ago

This PR adds full support for requesting profiles, maps and providers lists from the services.

Add the same time, the providers response interface was fixed. Interfaces for all (profiles, maps & providers) were moved to their own respective files from a single store interface module.


Easy to go through commit by commit.

kysely commented 3 years ago

During this, I noticed naming inconsistencies slowly creeping in.

E.g. fetchers for maps, profiles, projects and insights follow the same naming conventions (getMap, getProfile, getProject, getSDKConfiguration, ...) but provider fetchers are named weirdly findOneProvider and findAllProviders.

I think this comes from porting from services method names but it'd be nice if they were consistent in the client.

Do you think it's worth renaming to getProvider and getProvidersList at this moment?

janhalama commented 3 years ago

During this, I noticed naming inconsistencies slowly creeping in.

E.g. fetchers for maps, profiles, projects and insights follow the same naming conventions (getMap, getProfile, getProject, getSDKConfiguration, ...) but provider fetchers are named weirdly findOneProvider and findAllProviders.

I think this comes from porting from services method names but it'd be nice if they were consistent in the client.

Do you think it's worth renaming to getProvider and getProvidersList at this moment?

Yes I would rename getProvider and getProvidersList now. Also findAllProviders has now limit and is not returning all providers any more.

kysely commented 3 years ago

During this, I noticed naming inconsistencies slowly creeping in. E.g. fetchers for maps, profiles, projects and insights follow the same naming conventions (getMap, getProfile, getProject, getSDKConfiguration, ...) but provider fetchers are named weirdly findOneProvider and findAllProviders. I think this comes from porting from services method names but it'd be nice if they were consistent in the client. Do you think it's worth renaming to getProvider and getProvidersList at this moment?

Yes I would rename getProvider and getProvidersList now. Also findAllProviders has now limit and is not returning all providers any more.

Good, I renamed the methods and documented the changes are breaking in changelog. However search through all @superfaceai org repos didn't find any use of either of the methods, so the change should be in reality non-breaking.

Is it ok to be merged now? :)

kysely commented 3 years ago

Merging, will wait for #54 before releasing.