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: implement types schema option parameter #385

Merged

Conversation

scottenock
Copy link
Contributor

Hi @sindresorhus, I decided to pick up this feature request. I used your comments on this PR: #249 to help guide me on implementation requirements.

Resolves #210
related to #201

Let me know if you have any feedback!

@scottenock scottenock force-pushed the feat/pre-defined-types-schema branch from c477f6d to 419fc0e Compare May 26, 2024 11:31
@sindresorhus
Copy link
Owner

Thanks for working on this

base.d.ts Outdated Show resolved Hide resolved
base.d.ts Outdated Show resolved Hide resolved
base.d.ts Outdated Show resolved Hide resolved
base.d.ts Outdated Show resolved Hide resolved
test/parse.js Outdated Show resolved Hide resolved
test/parse.js Outdated Show resolved Hide resolved
base.d.ts Show resolved Hide resolved
@scottenock
Copy link
Contributor Author

Thanks for the feedback @sindresorhus! I've done a second pass of the doc comments and tried to get it as concise and clear as possible. Let me know if there's any areas you think need reworking 💪

base.d.ts Outdated Show resolved Hide resolved
base.d.ts Outdated Show resolved Hide resolved
base.js Outdated Show resolved Hide resolved
base.js Outdated Show resolved Hide resolved
base.js Outdated Show resolved Hide resolved
base.d.ts Outdated Show resolved Hide resolved
base.d.ts Outdated Show resolved Hide resolved
base.d.ts Show resolved Hide resolved
test/parse.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

It also needs to be added to the readme. I would wait until the TS docs are done. It should be as close to the TS docs as possible.

@scottenock
Copy link
Contributor Author

Thanks for taking another look at my changes @sindresorhus 💪

I've made the corrections, and have added additional context in the TS Docs. Let me know if they look good to you, and I'll get the readme updated accordingly.

@sindresorhus
Copy link
Owner

I've made the corrections, and have added additional context in the TS Docs. Let me know if they look good to you, and I'll get the readme updated accordingly.

Looks good 👍

@scottenock
Copy link
Contributor Author

That's great @sindresorhus , I've updated the readme.md and have adjusted it slightly to work with the different format of the file. Let me know what you think!

@sindresorhus sindresorhus merged commit 672eb82 into sindresorhus:main Jul 22, 2024
2 checks passed
@sindresorhus
Copy link
Owner

Thanks for contributing 🙏

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

Successfully merging this pull request may close these issues.

Support predefining types
2 participants