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

feat(api): improve nodejs api #323

Merged
merged 20 commits into from
Sep 22, 2021
Merged

Conversation

eduardomourar
Copy link
Contributor

In order to make somewhat more extensible to use library with Node.js, we will need to make the following changes:

  • Allow the input to be an object (a single schema or multiple schemas either with parsed content or its full path)
  • Make the output an plain object containing the readme, input schemas and markdown
  • Create TypeScript definition

Related to #300.

@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #323 (d52325f) into main (48335c3) will increase coverage by 0.74%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
+ Coverage   98.57%   99.31%   +0.74%     
==========================================
  Files          11       11              
  Lines         563      588      +25     
==========================================
+ Hits          555      584      +29     
+ Misses          8        4       -4     
Impacted Files Coverage Δ
lib/symbols.js 100.00% <ø> (ø)
lib/formatInfo.js 100.00% <100.00%> (ø)
lib/index.js 97.56% <100.00%> (+2.03%) ⬆️
lib/markdownBuilder.js 99.12% <100.00%> (ø)
lib/readmeBuilder.js 100.00% <100.00%> (ø)
lib/schemaProxy.js 100.00% <100.00%> (ø)
lib/writeMarkdown.js 100.00% <100.00%> (+10.00%) ⬆️
lib/writeSchema.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48335c3...d52325f. Read the comment docs.

Copy link
Collaborator

@trieloff trieloff left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, but I find it too hard to review the unrelated changes and disentangle the substance of the PR.

Please:

  • do not change the copyright dates
  • do not introduce camel case variable names (open a separate refactoring/style PR if it bothers you too much)
  • do not change import orders or import names (same as above)
  • make sure the test coverage does not go down

I will review then, as the substance of the PR seems to make sense, it's just hard to see underneath the mountain of other changes.

.eslintrc.js Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
cli.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
lib/formatInfo.js Show resolved Hide resolved
lib/formattingTools.js Outdated Show resolved Hide resolved
lib/keywords.js Outdated Show resolved Hide resolved
@eduardomourar eduardomourar force-pushed the feat/improve-node-api branch from 9ebff10 to 9839b78 Compare July 20, 2021 09:44
@eduardomourar eduardomourar requested a review from trieloff July 29, 2021 10:29
@trieloff
Copy link
Collaborator

trieloff commented Jul 30, 2021

Hi @eduardomourar - thanks for the update, I may need until the week after next to review this.

@eduardomourar
Copy link
Contributor Author

Hi @eduardomourar - thanks for the update, I may need until the week after next to review this.

No rush, we are doing extensive tests against our schemas in the meantime.

Copy link
Collaborator

@trieloff trieloff left a comment

Choose a reason for hiding this comment

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

Good, but the comment formatting and copyright dates need to stay the way they've been before.

.eslintrc.js Outdated Show resolved Hide resolved
test/formatting.test.js Outdated Show resolved Hide resolved
@trieloff trieloff merged commit 2a0f6f8 into adobe:main Sep 22, 2021
@trieloff
Copy link
Collaborator

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants