-
Notifications
You must be signed in to change notification settings - Fork 392
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
Feature/proctree query time #3691
Feature/proctree query time #3691
Conversation
@rafaeldtinoco WDYT? |
8071a92
to
ee9d11c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Needs a better reasoning and a PR changing the types before this can be merged. Im putting a -1 for now until both are satisfied. Request my review in the types change, please. I'll merge and you can rebase this and change types dependency to that commit.
@rafaeldtinoco I probably need your opinion on the following question: |
I think I prefer this idea better Alon, if you also think that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We agreed the location of Timestamp will be a struct called TimeRelevantInfo instead of duplicates under other Info structs.
ee9d11c
to
5a36b51
Compare
Fill the timestamp field of proctree info objects upon query.
5a36b51
to
5b93e61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
1. Explain what the PR does
7fa990f feat(proctree): fill timestamp of proctree info objects
Fix #3700
2. Explain how to test it
e2e test
3. Other comments