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

gateio: (minor) do not crash in parseOrder() on empty timestamp #3033

Merged
merged 1 commit into from
Jun 5, 2018
Merged

gateio: (minor) do not crash in parseOrder() on empty timestamp #3033

merged 1 commit into from
Jun 5, 2018

Conversation

egorFiNE
Copy link
Contributor

@egorFiNE egorFiNE commented Jun 4, 2018

No description provided.

@egorFiNE
Copy link
Contributor Author

egorFiNE commented Jun 4, 2018

Could you perhaps create label "minor" for changes that needs very simple and quick code review? Sort of like "minor edit" in wikipedia or something?..

@egorFiNE egorFiNE changed the title gateio: do not crash in parseOrder() on empty timestamp gateio: (minor) do not crash in parseOrder() on empty timestamp Jun 4, 2018
@kroitor kroitor self-assigned this Jun 4, 2018
@@ -378,7 +381,7 @@ module.exports = class gateio extends Exchange {
}
return {
'id': id,
'datetime': this.iso8601 (timestamp),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to understand why it did crash there, whereas it actually should've worked ok... the iso8601 method is designed to work with undefined values, unless there was some other type of value in the timestamp (not undefined, but null, maybe?)

@egorFiNE
Copy link
Contributor Author

egorFiNE commented Jun 4, 2018

It crashes in multiple exchanges and multiple cases.

I thought as well to update the base method. Shall we?

@egorFiNE
Copy link
Contributor Author

egorFiNE commented Jun 4, 2018

And null, yes. It does happen.

@kroitor
Copy link
Member

kroitor commented Jun 5, 2018

I thought as well to update the base method. Shall we?

That would be awesome!

@kroitor kroitor merged commit bef654a into ccxt:master Jun 5, 2018
@egorFiNE egorFiNE deleted the ef-gateio-datetime branch June 5, 2018 09:23
@egorFiNE
Copy link
Contributor Author

egorFiNE commented Jun 5, 2018

That would be awesome!

Here you go: #3043.

PHP and Python versions will follow today from my coworkers with similar branch names.

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

Successfully merging this pull request may close these issues.

None yet

2 participants