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

Change entry parentId to require #21

Closed

Conversation

haoadoreorange
Copy link

It seems to me that the parentId is always set to -1 if no parent, unless there's a case where it's actually undefined that I'm not aware of.

Signed-off-by: Quoc-Hao Tran [email protected]

@paul-marechal
Copy link
Member

Is it rather an implementation issue?

https://github.com/theia-ide/trace-server-protocol/blob/e3dfec64fea4b7f754ba67e4d24363712847b2b2/API.yaml#L2123-L2127

https://github.com/theia-ide/trace-server-protocol/blob/e3dfec64fea4b7f754ba67e4d24363712847b2b2/API.yaml#L2133-L2135

The OpenAPI spec seems to say it should be unset when there is no parent, and doesn't mention anything about valid/invalid parentId such as -1.

@haoadoreorange
Copy link
Author

Hmmm seems like you're right.
Any thoughts on how we should streamline this ? @MatthewKhouzam @bhufmann

@haoadoreorange
Copy link
Author

Any news on this ?

@bhufmann
Copy link
Collaborator

bhufmann commented Oct 18, 2021

BTW, @haoadoresorange you closed the issue.

So, in the TSP it's marked as optional to reduce the number of required fields to be serialized. The Trace Compass server always always serializes the parentId and internal in Trace Compass -1 means no parents. The theia-trace-extension implementation treats -1 the same way as undefined which means no parent. The handling of -1 is right now an unspecified behavior.

Looking at the TSP specification and front-end/back-implementation there is some clean-up needed, starting with the TSP. What is the specification for the parentId do we want? After the decision and update of the TSP (if needed) the back-end and front-end(s) need to be aligned to the specification. I opened the tracker eclipse-cdt-cloud/trace-server-protocol#65 for this discussion .

@haoadoreorange
Copy link
Author

Hi, it seems auto-closed at some point because I made some hacky changes on the branch...

As you said, the Trace Compass server always serialize it as -1 and I think this is the way to go, imho because it means no type exception ever because it's always int/number in almost every languages. The optional/undefined is not the same concept everywhere.

Unless there's some drawback with -1 or negative number for this field that I'm not aware of.

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.

3 participants