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.

Sign up for GitHub

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

Jump to bottom

Core, CSDL, and JSON Format: starter documents and tool tweaks #34

Merged
merged 35 commits into from
Jul 24, 2023
Merged

Core, CSDL, and JSON Format: starter documents and tool tweaks #34

merged 35 commits into from
Jul 24, 2023

Conversation

ralfhandl
Copy link
Contributor

@ralfhandl ralfhandl commented Jul 20, 2023 •
edited
Loading

Fixes #29
Fixes #33
Fixes #36
Fixes #38

Tool issues:

  • odata-json-format.html is generated empty
  • odata-data-aggregation-ext.md and odata-data-aggregation-ext.html are generated empty
  • generate PDF

Documents:

  • CSDL JSON starter
  • CSDL XML starter
  • JSON Format
  • Protocol starter
  • URL Conventions starter

@ralfhandl ralfhandl added the JSON Format JSON Format label Jul 20, 2023
@ralfhandl ralfhandl mentioned this pull request Jul 20, 2023
Closed
5 tasks
@ralfhandl ralfhandl changed the title JSON Format - starter document CSDL and JSON Format - starter documents and tool tweaks Jul 21, 2023
@ralfhandl ralfhandl added the Protocol Protocol, URL Conventions label Jul 21, 2023
@ralfhandl ralfhandl marked this pull request as ready for review July 21, 2023 12:43
@ralfhandl ralfhandl changed the title CSDL and JSON Format - starter documents and tool tweaks Core, CSDL and JSON Format: starter documents and tool tweaks Jul 21, 2023
@ralfhandl ralfhandl changed the title Core, CSDL and JSON Format: starter documents and tool tweaks Core, CSDL, and JSON Format: starter documents and tool tweaks Jul 21, 2023
GeraldKrause
GeraldKrause requested changes Jul 21, 2023
View reviewed changes
Copy link
Contributor

@GeraldKrause GeraldKrause left a comment

Choose a reason for hiding this comment

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

Here's feedback on the document-related files.
Havent looked yet at the code in lib

odata-json-format/meta.yaml Show resolved Hide resolved
odata-csdl-xml/meta.yaml Show resolved Hide resolved
odata-csdl-xml/odata-csdl-xml-v4.02-csd01.md Outdated Show resolved Hide resolved
odata-csdl-json/meta.yaml Show resolved Hide resolved
odata-csdl-json/odata-csdl-json-v4.02-csd01.md Outdated Show resolved Hide resolved
GeraldKrause
GeraldKrause reviewed Jul 24, 2023
View reviewed changes
Copy link
Contributor

@GeraldKrause GeraldKrause left a comment

Choose a reason for hiding this comment

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

LGTM

lib/number.js Outdated
}

// If all parts are equal, compare the lengths of the section numbers
return partsA.length - partsB.length;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can there ever be a result different from 0?

Copy link
Contributor Author

@ralfhandl ralfhandl Jul 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Assume files 3 foo.md and 3.2 bar.md, in this case the lengths of partA = ["3"] and partB = ["3", "2"] differ.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I add a unit test for function compareSectionNumbers?

GeraldKrause
GeraldKrause approved these changes Jul 24, 2023
View reviewed changes
Copy link
Contributor

@GeraldKrause GeraldKrause left a comment

Choose a reason for hiding this comment

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

LVGTM

@ralfhandl ralfhandl merged commit 1fbc0bf into main Jul 24, 2023
1 check passed
@ralfhandl ralfhandl deleted the json-format branch July 24, 2023 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GeraldKrause GeraldKrause GeraldKrause approved these changes

Assignees
No one assigned
Labels
JSON Format JSON Format Protocol Protocol, URL Conventions tools
Projects
None yet
Milestone
No milestone
2 participants
@ralfhandl @GeraldKrause