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

fix: add type declarations #113

Merged
merged 15 commits into from
Jan 9, 2022
Merged

fix: add type declarations #113

merged 15 commits into from
Jan 9, 2022

Conversation

RahulGautamSingh
Copy link
Contributor

@RahulGautamSingh RahulGautamSingh commented Jan 4, 2022 •
edited
Loading

Changes:

Adds type declarations for the package.

Context:

@RahulGautamSingh
Copy link
Contributor Author

@rarkins Could you approve the workflows please..

@RahulGautamSingh
Copy link
Contributor Author

@rarkins Could you approve the workflows again please..

@RahulGautamSingh RahulGautamSingh mentioned this pull request Jan 7, 2022
Merged
6 tasks
@rarkins
Copy link
Contributor

rarkins commented Jan 7, 2022

@viceice could you review?

viceice
viceice reviewed Jan 7, 2022
View reviewed changes
lib/version.d.ts Outdated Show resolved Hide resolved
viceice
viceice reviewed Jan 7, 2022
View reviewed changes
index.d.ts Outdated Show resolved Hide resolved
lib/operator.d.ts Outdated Show resolved Hide resolved
lib/semantic.d.ts Outdated Show resolved Hide resolved
lib/shared.ts Outdated Show resolved Hide resolved
lib/specifier.d.ts Outdated Show resolved Hide resolved
lib/version.d.ts Show resolved Hide resolved
RahulGautamSingh and others added 6 commits January 7, 2022 15:27
@viceice
Update lib/version.d.ts
866765e 
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
@viceice
Update index.d.ts
fd3f850 
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
@viceice
Update lib/semantic.d.ts
7568f8c 
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
@viceice
Update lib/operator.d.ts
669b698 
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
@viceice
Update lib/shared.ts
08fed96 
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
@viceice
Update lib/specifier.d.ts
7d8bb7c 
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
@RahulGautamSingh
Copy link
Contributor Author

Ready for review and merge.🟩

viceice
viceice reviewed Jan 7, 2022
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

Why are some functions have a declare? They should have all a declare or none.

@RahulGautamSingh
Copy link
Contributor Author

Why are some functions have a declare? They should have all a declare or none.

Resolved

@RahulGautamSingh
Copy link
Contributor Author

RahulGautamSingh commented Jan 8, 2022 •
edited
Loading

Ready for review.🟩
For some reason the request-review btn is not working for this PR

viceice
viceice reviewed Jan 8, 2022
View reviewed changes
lib/specifier.d.ts Outdated Show resolved Hide resolved
viceice
viceice reviewed Jan 8, 2022
View reviewed changes
lib/shared.ts Outdated Show resolved Hide resolved
@viceice
Copy link
Member

viceice commented Jan 8, 2022

@rarkins can you check my permission on this repo?

RahulGautamSingh and others added 2 commits January 9, 2022 16:15
@viceice
Update lib/specifier.d.ts
6d67e52 
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
Rename shared.ts to shared.d.ts
1362e9d
@RahulGautamSingh
Copy link
Contributor Author

Ready.

@viceice viceice mentioned this pull request Jan 9, 2022
Closed
@viceice
Copy link
Member

viceice commented Jan 9, 2022

You need to add the index.d.ts to files in package.json

viceice
viceice approved these changes Jan 9, 2022
View reviewed changes
package.json
@@ -51,7 +51,8 @@
},
"files": [
"index.js",
"lib"
"lib",
"index.d.ts"
Copy link
Member

Choose a reason for hiding this comment

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

If this is missing, the file won't included in published package. 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I read somewhere that all files are included by default now, and only the files under .npmignore are left out.

Nonetheless this is a good code-practice and an advisable thing to do as per the docs.✨

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, by default all files are included. But if you define the files explicit, only those will be included in published package.

Copy link
Contributor Author

@RahulGautamSingh RahulGautamSingh Jan 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Ahh yes, got it. I misunderstood and thought that even if we explicitly have a files property all others will be included.
Thanks , more clarity now.🙏

@viceice viceice merged commit eefd68b into renovatebot:main Jan 9, 2022
@github-actions
Copy link

🎉 This PR is included in version 1.1.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
Reviewers

@viceice viceice viceice approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RahulGautamSingh @rarkins @viceice