Skip to content

Comments

Extend the calendar day with session and settlement info#268

Open
gnvk wants to merge 1 commit intomasterfrom
ext-calendar
Open

Extend the calendar day with session and settlement info#268
gnvk wants to merge 1 commit intomasterfrom
ext-calendar

Conversation

@gnvk
Copy link
Collaborator

@gnvk gnvk commented Dec 6, 2023

Fixes #266

@gnvk gnvk requested review from neal and sai-g December 6, 2023 07:13
@gnvk gnvk changed the title Extend the calendar day with session and settlment info Extend the calendar day with session and settlement info Dec 6, 2023
Comment on lines +181 to +182
SessionOpen string `json:"session_open"`
SessionClose string `json:"session_close"`
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't like to add the session open/close fields here as they were exposed to the API unintentionally and there is no good use for them either as they don't really mean much as far as trading goes.

We can't remove it from the API response as that would be a breaking change, but at least we avoid adding it here to avoid more breaking changes in future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you please add this comment to the original issue: #266?

@sai-g
Copy link
Contributor

sai-g commented Mar 3, 2025

@gnvk can we close this PR?

@gnvk
Copy link
Collaborator Author

gnvk commented Mar 4, 2025

@gnvk can we close this PR?

There's a kind of stuck discussion about this under this issue: #266 It would be nice to find a solution there.

@DrudTheTerrible
Copy link

@gnvk can we close this PR?

There's a kind of stuck discussion about this under this issue: #266 It would be nice to find a solution there.

Issue #266 hasn't been updated in over a year. I think it's fine to close both the issue and PR if there is no intention to resolve it in short order.

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.

Additions to Calendar struct

4 participants