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

PKG -- [util-address] Generate types from JSDoc #1547

Merged
merged 8 commits into from
Feb 3, 2023

Conversation

chasefleming
Copy link
Member

Addresses: #1537, #1536

@chasefleming chasefleming requested a review from a team as a code owner February 1, 2023 23:30
@changeset-bot
Copy link

changeset-bot bot commented Feb 1, 2023

🦋 Changeset detected

Latest commit: 36459af

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@onflow/util-address Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -0,0 +1 @@
{"version":3,"file":"index.d.ts","sourceRoot":"","sources":["../src/index.js"],"names":[],"mappings":"AAAA;;;;GAIG;AACF,oCAHU,MAAM,GACJ,MAAM,CAKlB;AAED;;;;GAIG;AACH,oCAHW,MAAM,GACJ,MAAM,CAKlB;AAED;;;;GAIG;AACH,iCAHW,MAAM,GACJ,MAAM,CAIlB"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be checked in?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was wondering the same thing. Can remove if that's what everyone thinks.

* @param {string} address - Flow address
* @returns {string} - Flow address without 0x prefix
*/
export function sansPrefix(address: string): string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Given this is a build product akin to the dist folder can we ignore these files in .gitignore?

Copy link
Member Author

Choose a reason for hiding this comment

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

It would make the build step easier, but don't most libraries have their type files committed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not if they are generated. Usually, they go into the dist folder.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you do find an example of what we are doing here out in the wild, I would be curious to see it.

Copy link
Contributor

@justinbarry justinbarry left a comment

Choose a reason for hiding this comment

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

Looks good! Left a few comments. If we can move the generated files out the repo it should make it simpler to keep the build in sync.

Copy link
Contributor

@justinbarry justinbarry left a comment

Choose a reason for hiding this comment

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

LGTM

@chasefleming chasefleming merged commit 2d143bc into master Feb 3, 2023
@chasefleming chasefleming deleted the chasefleming/type-util-address branch February 3, 2023 17:14
@github-actions github-actions bot mentioned this pull request Jun 2, 2023
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.

2 participants