Skip to content

Conversation

@siwei-zhang-work
Copy link
Contributor

What it does

It fixes problems in swagger documentation introduced in:#203

  • Fixed "xValuesDescription" and "yValuesDescription" to show XYAxisDescription instead of arrays.
  • Renamed "AxisDomainTimeRange" to "AxisDomainRange" to show that it is not only for time ranges. Previously "IAxisDomain.TimeRange" was renamed to "IAxisDomain.Range" but swagger documentation wasn't updated accordingly.

How to test

Start the server and browse to http://localhost:8080/tsp/api/openapi.yaml

Follow-ups

No follow-ups.

Review checklist

  • [Y] As an author, I have thoroughly tested my changes and carefully followed the instructions in this template

bhufmann
bhufmann previously approved these changes Aug 20, 2025
Copy link
Contributor

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

@JsonSubTypes({
@JsonSubTypes.Type(value = AxisDomainCategorical.class, name = "categorical"),
@JsonSubTypes.Type(value = AxisDomainTimeRange.class, name = "timeRange")
@JsonSubTypes.Type(value = AxisDomainRange.class, name = "range")
Copy link
Contributor

Choose a reason for hiding this comment

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

The change from 'timeRange' to 'range' should also be applied to the AxisDomainSerializer.

 - Fixed "xValuesDescription" and "yValuesDescription" to show
   XYAxisDescription instead of arrays.
 - Renamed "AxisDomainTimeRange" to "AxisDomainRange" to show
   that it is not only for time ranges. Previously
   "IAxisDomain.TimeRange" was renamed to "IAxisDomain.Range"
   but swagger documentation wasn't updated accordingly.
Copy link
Contributor

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@bhufmann
Copy link
Contributor

The build error is unrelated. I'll go ahead and merge it. I keep on eye on the build.

@bhufmann bhufmann merged commit 0cae21c into eclipse-tracecompass-incubator:master Aug 21, 2025
1 of 5 checks passed
@bhufmann
Copy link
Contributor

The build error is unrelated. I'll go ahead and merge it. I keep on eye on the build.

It turns out that the error was caused by this PR. I missed that and should not have merged the PR. PR #217 fixes the build issue.

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