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: add an ability to parse json encoded options #192

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

paulish
Copy link
Contributor

@paulish paulish commented Oct 4, 2021

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • [+] Feature
  • Test
  • Docs
  • Refactor
  • Build-related changes
  • Other, please describe:

Description:

I tried to solve the issue #181 using your coding style.

@kevinccbsg kevinccbsg added the hacktoberfest-accepted Accepted for hacktoberfest, will merge later label Oct 7, 2021
Copy link
Member

@kevinccbsg kevinccbsg left a comment

Choose a reason for hiding this comment

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

I really like the implementation @paulish 👏 I was wondering if in future PR we could do something like this.

/**
  * A song
  * @typedef {object} Song
  * @property {string} title - The title
  * @property {string} artist - The artist - json:{"maxLength": 300}
  * @property {number} year - The year - int64 - json:{
  *     "minimum": 2000
  *   }
  */

Like a multiline.

However, the PR looks good to me. Let's wait until @LonelyPrincess or @bri06 ✅ to merge this.

@paulish
Copy link
Contributor Author

paulish commented Oct 8, 2021

@kevinccbsg I agree that multiline would be a nice addition but it requires more changes to the library and perhaps require some redesign. If this change is accepted we can go ahead and try to implement multiline.

@kevinccbsg kevinccbsg merged commit f8a39e7 into BRIKEV:master Oct 11, 2021
@kevinccbsg
Copy link
Member

@all-contributors please add @paulish for code

@allcontributors
Copy link
Contributor

@kevinccbsg

I've put up a pull request to add @paulish! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted for hacktoberfest, will merge later
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants