Skip to content

Pattern support#249

Merged
erunion merged 18 commits intoreadmeio:mainfrom
garrett-wade:patternSupport
Mar 2, 2022
Merged

Pattern support#249
erunion merged 18 commits intoreadmeio:mainfrom
garrett-wade:patternSupport

Conversation

@garrett-wade
Copy link
Copy Markdown
Contributor

@garrett-wade garrett-wade commented Mar 1, 2022 •
edited by erunion
Loading

Adding support for custom comment pattern specification

This is initial enhancement to support the following issue readmeio/oas#603.

This enhancement:

  • added an optional pattern argument command-line argument with help text
  • added pattern attribute and get/set methods to Options object
  • added loadPattern method to Loader to load json file that contains specification of pattern
  • added section in swaggerInline in index file to handle adding the loaded pattern file to the options variable
  • ensured backwards compatibility if no pattern is specified
  • expanded test coverage to cover all changes

Testing

Refer to the can extract comments from custom pattern and custom file test in the extractor.test.js file to see a detailed test of the additional functionality.

@garrett-wade
feat(extractor): add support for custom comment patterns
d8fa8dd 
- added pattern command-line arguement
- added pattern attribute to Options object
@garrett-wade
feat(extractor): add support for custom comment patterns
bb0f56a 
- added pattern command-line arguement with help text
- added pattern attribute and get/set methods to Options object
- added loadPattern method to Loader to load json file
- added section in swaggerInline in Indx to handle adding the loaded pattern file to options
- expanded test coverage to cover all changes
@garrett-wade
Merge branch 'patternSupport' of https://github.com/garrett-wade/swag…
d285ff8 
…ger-inline into patternSupport
@erunion erunion added the enhancement New feature or request label Mar 1, 2022
erunion
erunion requested changes Mar 1, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

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

Handful of nits, but overall this looks pretty solid! Thanks for picking this up.

Can you also re-run Prettier on your changes? You can run it with npx prettier --list-different --write "./**/**.{js}"

Comment thread src/cli.js Outdated
Comment thread __tests__/extractor.test.js Outdated
Comment thread __tests__/extractor.test.js Outdated
Comment thread src/extractor.js Outdated
Comment thread src/extractor.js Outdated
Comment thread src/index.js Outdated
Comment thread src/loader.js Outdated
@garrett-wade
Copy link
Copy Markdown
Contributor Author

garrett-wade commented Mar 1, 2022

@erunion Receiving following error when running prettier: [error] No files matching the pattern were found: "./**/**.{js}". Any thoughts here?

@erunion
Copy link
Copy Markdown
Member

erunion commented Mar 1, 2022

@garrett-wade I just pushed up c718583 that has a prettier script so you can run npm run prettier instead. Works locally for me so hopefully should for you.

@garrett-wade
Copy link
Copy Markdown
Contributor Author

garrett-wade commented Mar 1, 2022

@erunion was able to run

erunion
erunion requested changes Mar 1, 2022
View reviewed changes
Comment thread __tests__/loader.test.js Outdated
Comment thread package.json
erunion
erunion requested changes Mar 2, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

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

Sorry, I finally had a chance to run this locally and a found a couple things. These should be the last of the changes! Thanks again for this.

Comment thread __tests__/loader.test.js Outdated
Comment thread __tests__/loader.test.js Outdated
Comment thread __tests__/loader.test.js Outdated
Comment thread __tests__/loader.test.js Outdated
Comment thread src/extractor.js
Comment thread src/index.js Outdated
Comment thread src/loader.js Outdated
garrett-wade and others added 6 commits March 2, 2022 08:04
@garrett-wade @erunion
Update __tests__/loader.test.js
f93c294 
Co-authored-by: Jon Ursenbach <[email protected]>
@garrett-wade @erunion
Update __tests__/loader.test.js
1122dc3 
Co-authored-by: Jon Ursenbach <[email protected]>
@garrett-wade @erunion
Update __tests__/loader.test.js
78fbfe8 
Co-authored-by: Jon Ursenbach <[email protected]>
@garrett-wade @erunion
Update __tests__/loader.test.js
be92048 
Co-authored-by: Jon Ursenbach <[email protected]>
@garrett-wade @erunion
Update src/index.js
419d8f5 
Co-authored-by: Jon Ursenbach <[email protected]>
@garrett-wade @erunion
Update src/loader.js
f2d0e7b 
Co-authored-by: Jon Ursenbach <[email protected]>
@garrett-wade
Copy link
Copy Markdown
Contributor Author

garrett-wade commented Mar 2, 2022

@erunion thanks for all of the feedback! This is my first time contributing to an open source library as well as extending a node.js library in general. Thank you for taking the time to make the suggestions, learned a lot!

erunion
erunion approved these changes Mar 2, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@erunion erunion left a comment

Choose a reason for hiding this comment

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

Thanks again for this!

@erunion erunion merged commit 2caec29 into readmeio:main Mar 2, 2022
@erunion
Copy link
Copy Markdown
Member

erunion commented Mar 2, 2022

Published to https://github.com/readmeio/swagger-inline/releases/tag/5.1.0

@erunion erunion mentioned this pull request Mar 2, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erunion erunion erunion approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@garrett-wade @erunion