search

Andargor / edce-client

Elite Dangerous Companion Emulator
MIT License
49 stars 13 forks source link

Updated api #27

Closed benpage26 closed 7 years ago

benpage26 commented 7 years ago

looks like the API has split data out into two endpoints /profile and /market

I've tried to merge the data from each of these together, but it looks like uploading the data doesn't work and I'm not sure why... (upload url is unresponsive)

benpage26 commented 7 years ago

Sorry, changes look horrible because I did a reformat...

This link shows changes with the whitespace suppressed. https://github.com/Andargor/edce-client/pull/27/files?w=1

Andargor commented 7 years ago

Hey there. I'm not clear on the state of the pull request, is it functional or something is still missing?

Andargor commented 7 years ago

I did a quick review of the code, and my comment is that I would split out the profile and market query handling into 2 functions, e.g. performProfileQuery and performMarketQuery, or a generic performQuery with the top level node as parameter. This in order to have a standard function other devs can use (since this is the purpose of EDCE).

Submission can then be done for the market data, and not sure if it's worth writing to file. @AnthorNet I'm guessing some third party tools like EDSM use the profile information, not just the market, and that it still needs to be submitted to EDDN?

EDIT: I'm willing to inspire myself from @benpage26 's code and rework it a little, I just need a bit of time over the weekend

Andargor commented 7 years ago

I've decided to revamp how the code works, now the three endpoints get concatenated into one object, it simplifies handling for our purposes. Please refer to version 2.0.0

Thanks Ben for the pull request.

AnthorNet commented 7 years ago

Just noticed the email, I'm not sure what we take from the profile page ^^ Now that we have the journal, almost every profile data comes from it.

Andargor commented 7 years ago

@AnthorNet No problem. Long-term, the journal is the way to go anyway for anything requiring authentication.