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

correct schema error #111

Closed
wants to merge 2 commits into from
Closed

correct schema error #111

wants to merge 2 commits into from

Conversation

wandmagic
Copy link
Contributor

Committer Notes

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you squashed any non-relevant commits and commit messages? [instructions]
  • Do all automated CI/CD checks pass?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you included examples of how to use your new feature(s)?
  • Have you updated all website and readme documentation affected by the changes you made? Changes to the website can be made in the website/content directory of your branch.

@wandmagic wandmagic requested a review from a team as a code owner March 13, 2024 22:07
Copy link

XSpec Test Results

  2 files  ±0  40 suites  ±0   0s ⏱️ ±0s
105 tests ±0  90 ✅ ±0  15 💤 ±0  0 ❌ ±0 
114 runs  ±0  99 ✅ ±0  15 💤 ±0  0 ❌ ±0 

Results for commit 9510963. ± Comparison against base commit 0010340.

Comment on lines 39 to 41
<field>
<use-name>overloaded</use-name>
</field>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain a little more in the comment or issue what this should be accomplishing? A field "by ref" that is @refless should not be allowed and should not work, but I guess I would need to know more context to understand why this change is valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when the JSON Schema is generated, the ref that is specified in the other overloaded fields doesn't exist causing an error. this field is so that the ref parameters in the other fields actually point to something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed that part and we can fix it with the other PR with the json schema testing

@wandmagic wandmagic closed this Mar 18, 2024
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.

2 participants