This request is in response to issue #658. #671
Conversation
…llowing changes were done:
- Added 'required' keyword to all code samples in the 'Define properties' section
- Reworded and restructured the 'Define properties' section for better understanding, and removed the 'Add the properties object' sub-section because it was unnecessary.
- Added an infobox to give more information about optional properties
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to the JSON Schema Community. Thanks a lot for creating your first pull request!! 🎉🎉 We are so excited you are here! We hope this is only the first of many! For more details check out README.md file.
chidiadi01
changed the title
benjagm
added
the
Status: In Progress
@gregsdennis, that means I have to wait for @bhavukkalra to respond? |
|
Hey No Actually. You don't need to wait for me .We did had a discussion on this change was agreed upon that we should be adding diff changes to the Json entries. I am already in the process of completing that change. Works on my local but getting inconsistent results across different pages. trying to debug that out Also, I think we use Info box currently to differentiate amongst different draft versions. Like If a certain keyword is not/is available in the draft and adding that as a generic info here could create confusion amongst readers. I think that's what greg was trying to instantiate as well that a discussion should happen on what changes are currently in sync and which are not. So that a PR could be accurately reviewed |
|
@chidiadi01 what I mean is that there's no discussion in the linked issue. It's just a proposal. There shouldn't be a PR from just a proposal. The issue needs to be discussed first to verify that s change needs to actually happen. Pre-emptively creating a PR is generally a waste of time because the work to be done hasn't been decided. The discussion that @bhavukkalra is taking about happened only this week during the OCWM call, which was after I left that comment. |
|
Oh...ok. Noted. |
|
Closing this PR as per #671 (comment) |
What kind of change does this PR introduce?
In this commit, the following changes were made:
Issue Number:
Screenshots/videos:
