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

Renaming/chaning JSON properties #30

Open
coonmoo opened this issue Jul 7, 2018 · 4 comments
Open

Renaming/chaning JSON properties #30

coonmoo opened this issue Jul 7, 2018 · 4 comments

Comments

@coonmoo
Copy link

coonmoo commented Jul 7, 2018

Hello,

I followed the commits in the last few days:

For example:

656209d

Why would you rename "damage_done_in_hand" to "damage_inhand"?
This would cause a lot of hassle, if you redeploy the API it would suddenly break our production website and services. Because we don't know when you deploy the API changes we will end up with missing data in our internal database.

Furthermore we would like to release a NuGet package for the API, if you are changing namings we'
d have to redeploy the package and all users of the package will have to update the package.

I don't see any benefit in renaming this property?

I know the API is still a pre-release, but I think a lot of people today are already using them actively.

@apugh
Copy link
Owner

apugh commented Jul 7, 2018

It wasn't renamed, but rather shortened and ADDED to getPlayerStats().

But I do see it is inconsistent with getPlayerMatchHistory(), which has had a damage_done_in_hand for some time.

I'll rework damage_inhand to be damage_done_in_hand for getPlayerStats() on Monday to make them consistent.

@coonmoo
Copy link
Author

coonmoo commented Jul 7, 2018

Sorry for the hassle! I should be able to read the commit messages more carefully :)

@luissilva1044894
Copy link
Contributor

Why now all json objects are lowcase? Against PaladinsAPI / SmiteAPI being CamelCase... And you removed [] from RealmAPI, but it will be sent on PaladinsAPI / SmiteAPI

Can you start to use "Patterns" for JSON / XML response?
Actually, isn't easy to code wrappers for Hi-Rez API. The same method will return different result / objects...

Example

CreateSession ()

RealmAPI:

{
	"created_datetime": "6/7/2018 3:14:46 PM",
	"id": 182379,
	"last_login_datetime": "7/5/2018 7:09:58 AM",
	"level": 31,
	"name": "Mittow",
	"region": "Brazil",
	"ret_msg": null,
	"steam_id": 76561198000421547
}

PaladinsAPI:

[
	{
		"Created_Datetime": "12/7/2016 12:30:05 AM",
		"Id": 5491799,
		"Last_Login_Datetime": "7/4/2018 8:32:56 PM",
		"Level": 332,
		"Name": "Mittow",
		"Platform": "Steam",
		"Region": "Brazil",
		"ret_msg": null
	}
]

GetPlayerStatus

RealmAPI:

{
	"match_id": 1319450,
	"personal_status_message": null,
	"ret_msg": null,
	"status": "In Game",
	"status_id": 3
}

PaladinsAPI:

[
	{
		"Match": 255022336,
		"match_queue_id": 428,
		"personal_status_message": null,
		"ret_msg": null,
		"status": 3,
		"status_string": "In Game"
	}
]

Can you accept my feedback #29 pull request?

@apugh
Copy link
Owner

apugh commented Jul 9, 2018

[QUESTION]: Why now all json objects are lowcase? Against PaladinsAPI / SmiteAPI being CamelCase... And you removed [] from RealmAPI, but it will be sent on PaladinsAPI / SmiteAPI

[ANSWER]: The patterns for Realm API are differing from that of SMITE/Paladins because there have been requests from the API community to remove redundancy, waste, etc. Areas of improvement have been removing the containing "List" objects, and moving common data to top level objects that subsequently contain Lists.

As for CamelCase ... it wasn't uniformly applied so I just want to lowercase everywhere with underscore separators.

Form follows function .. immediate goals are to efficiently get meaningful data, that is correct, out of the API. I'm OK with polishing format, etc. but don't want to create churn for API Devs that are already adapting to these defacto "standards" as functionality is added.

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

3 participants