esi / esi-issues

Issue tracking and feature requests for ESI
https://esi.evetech.net/
207 stars 23 forks source link

Consistency: solar_system_id vs system_id #1029

Open Blacksmoke16 opened 5 years ago

Blacksmoke16 commented 5 years ago

A quick look through the swagger spec shows that 17 endpoints are using system_id and 13 are using solar_system_id. I propose we decide on one and use that consistently throughout all the endpoints.

I can compile a list of ones that would need to change once we decide on which to use.

I personally would use system_id

solar_system_id

system_id

GoldenGnu commented 5 years ago

system_id have my vote too

ccp-zoetrope commented 5 years ago

system_id is preferred moving forward. Thanks for flagging this.

ghost commented 5 years ago

I recommend against "system_id" for exactly the reason that caused this bug: https://github.com/esi/esi-issues/issues/966

The word "system" generally means "a software system" in CCP code. Changing it to mean "solar system" on the ESI side risks more of the above confusion. Specificity is good.

Blacksmoke16 commented 5 years ago

I updated the OP with endpoints that use each.

guiguilechat commented 5 years ago

"solar_id" would be better.

"system" is a very broad term and should not be used alone. Using it in development can lead to misunderstanding what a name stands for, or even in bugs if people are not sanitizing the names, eg when creating the structures to represent the result (System is a package in java, that represents the jvm)

"solar_system_id" is too long.

if "solar_id" can't do it, then "solar_system_id". But calling a solar system "system" is a way to include bugs later. Don't call your variable/structure/clas/etc. "system".

kennethjor commented 5 years ago

The systems endpoint itself returns system_id, so this should be the standard.