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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the JSON Schema Documentation page #1030

Merged
merged 9 commits into from Sep 12, 2019

Conversation

@sladyn98
Copy link
Contributor

sladyn98 commented Aug 28, 2019

Changing the doc to include the Beta Release of JSON Schema

Your checklist for this pull request

馃毃 Please review the guidelines for contributing to this repository.

  • [x ] Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • [ x] Ensure that the pull request title represents the desired changelog entry
  • [ x] Please describe what you did
  • [ x] Link to relevant issues in GitHub or in Jenkins JIRA
  • [ x] Link to relevant pull requests, esp. upstream and downstream changes
  • [ x] Did you provide a test-case? That demonstrates feature works or fixes the issue.
@sladyn98

This comment has been minimized.

Copy link
Contributor Author

sladyn98 commented Aug 28, 2019

@oleg-nenashev @casz This should do it right?

Copy link
Member

oleg-nenashev left a comment

PR checklist is not really followed, not a consumable chamgelog entry I would also suggest to start creating a new documentation page for schema instead of keeping it on landing. Will help with future changes

@sladyn98

This comment has been minimized.

Copy link
Contributor Author

sladyn98 commented Aug 28, 2019

@oleg-nenashev

This comment has been minimized.

Copy link
Member

oleg-nenashev commented Aug 29, 2019

@sladyn98

This comment has been minimized.

Copy link
Contributor Author

sladyn98 commented Aug 30, 2019

@oleg-nenashev @casz This looks fine ?

Copy link
Member

oleg-nenashev left a comment

Please add title to the page and coordinate the merge with the feature release.
Also, my comment about the pull request title has not been addressed yet

docs/features/jsonSchema.md Outdated Show resolved Hide resolved
docs/features/jsonSchema.md Outdated Show resolved Hide resolved
docs/features/jsonSchema.md Outdated Show resolved Hide resolved
@sladyn98 sladyn98 changed the title Beta Release Announcement JSONSchema Document Page. Aug 30, 2019
@sladyn98 sladyn98 force-pushed the sladyn98:Beta_Release_schema branch from 84c9e45 to 5bae285 Sep 1, 2019
Copy link
Member

oleg-nenashev left a comment

  • Link from the root pages is missing
  • There is no reference to the old JSON generation and the differences. IMHO it should be documented somehow with proper disclaimers
docs/features/jsonSchema.md Outdated Show resolved Hide resolved
docs/features/jsonSchema.md Outdated Show resolved Hide resolved
@sladyn98 sladyn98 force-pushed the sladyn98:Beta_Release_schema branch from 6b20149 to 1eaad57 Sep 2, 2019
docs/features/jsonSchema.md Show resolved Hide resolved
docs/features/jsonSchema.md Show resolved Hide resolved
Co-Authored-By: Oleg Nenashev <o.v.nenashev@gmail.com>
Copy link
Member

oleg-nenashev left a comment

ready to go once the feature is integrated

Copy link
Member

oleg-nenashev left a comment

Updates are needed after recent changes in #980

@sladyn98 sladyn98 requested a review from oleg-nenashev Sep 12, 2019
docs/features/jsonSchema.md Outdated Show resolved Hide resolved
@oleg-nenashev oleg-nenashev changed the title JSONSchema Document Page. Add the JSON Schema Documentation page Sep 12, 2019
Copy link
Member

oleg-nenashev left a comment

The page still need to be referenced from README. The rest looks good to me

docs/features/jsonSchema.md Outdated Show resolved Hide resolved
docs/features/jsonSchema.md Outdated Show resolved Hide resolved
docs/features/jsonSchema.md Outdated Show resolved Hide resolved
docs/features/jsonSchema.md Outdated Show resolved Hide resolved
@sladyn98

This comment has been minimized.

Copy link
Contributor Author

sladyn98 commented Sep 12, 2019

Please add title to the page and coordinate the merge with the feature release.
Also, my comment about the pull request title has not been addressed yet

@oleg-nenashev Yeah added it :)

@sladyn98 sladyn98 requested a review from oleg-nenashev Sep 12, 2019
@sladyn98 sladyn98 requested a review from timja Sep 12, 2019
README.md Outdated Show resolved Hide resolved
@sladyn98 sladyn98 requested a review from oleg-nenashev Sep 12, 2019
@oleg-nenashev oleg-nenashev requested a review from casz Sep 12, 2019
@casz
casz approved these changes Sep 12, 2019
@oleg-nenashev oleg-nenashev merged commit 17de1bb into jenkinsci:master Sep 12, 2019
3 of 4 checks passed
3 of 4 checks passed
LGTM analysis: Java No code changes detected
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can鈥檛 perform that action at this time.