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

0.43.0 Potential Finishing Touches #17

Merged
merged 18 commits into from
Mar 20, 2024
Merged

0.43.0 Potential Finishing Touches #17

merged 18 commits into from
Mar 20, 2024

Conversation

typhonrt
Copy link
Contributor

@typhonrt typhonrt commented Mar 2, 2024

Hi @brettz9.

So this PR contains "finishing touches" to potentially tighten up the next release even more. This includes changes to how type declarations are built (via esm-d-ts / now bundled ./dist/index.d.ts), adds API docs in ./docs suitable to host on Github pages, tightened up README links to local repo (IE contributors links and others) / added badges, and updated the Github CI / CD action.

It's recommended to:

  • Enable Github actions for automatic tests on commit.
  • Enable Github pages for the ./docs directory.

In the repo settings for Github pages simply select ./docs:
image

The badges in the README for CI/CD and API documentation will start working once the above is done.

This is what the API docs will look like when enabled in the main repo

All that is remaining is a review, potentially merge this PR, and then bump the version in package.json and release.

@typhonrt
Copy link
Contributor Author

typhonrt commented Mar 2, 2024

Both of these CI / CD / testing related problems are now fixed.

Interestingly enough. I ran into this issue with eslint-plugin-jsdoc#1114 for the CI / CD pipeline / GH action testing. It looks like eslint-plugin-jsdoc doesn't know how to parse the exports field in packages.json to find the types associated with a package for the imports-as-dependencies rule. I originally removed the types field in package.json as it is defined in exports, but currently eslint-plugin-jsdoc requires a types field for that rule. Right now I just added back the types field.

I also expanded the testing matrix to cover Windows, Ubuntu, Mac. I'm just tracking down the checkout issue in CI / GH action regarding the LF / CRLF line endings when files are checked out for Windows in CI which fails the eslint rule: @stylistic/linebreak-style on Windows. I think it's good to run the tests over Node 16, 18, 20 and these platforms.
To fix this I added a .gitattributes file to check out all text files with LF line endings.

The good news on the latter is that it should now be possible to check out the repo for anyone working on Windows that might contribute to this project.

…encies`. This package is referenced in the type declarations.
@brettz9
Copy link
Contributor

brettz9 commented Mar 8, 2024

May take me maybe another week or so to get to this.

@typhonrt
Copy link
Contributor Author

No worries. A lot of this PR is just tidying up w/ links in the README and then the declarations / docs. This small delay has allowed me to get full test coverage going for esm-d-ts / IE the consumer on my side for jsdoccomment.

Copy link
Contributor

@brettz9 brettz9 left a comment

Choose a reason for hiding this comment

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

Though I haven't kicked the tires yet with a local install, it LGTM besides the changes requested...

tsconfig.json Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/jsdoccomment.js Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
@typhonrt
Copy link
Contributor Author

typhonrt commented Mar 16, 2024

Based on the discussion above I have added two more commits that:

  • Add back typescript in devDependencies so tsc can be run for type checking.
    • Added the tsc script back.
    • Modified tsconfig.json adding target and skipLibCheck for tsc checking of src / test.

New changes:

  • Removed .npmignore in favor of an explicit files list in package.json. Just /dist, /src, CHANGES.md, and LICENSE-MIT.txt need to be distributed.

  • Moved @types/eslint and @types/estree to dependencies as they are referenced in the public declarations via jsdoccomment.js.

For reference here is what the distribution looks like when installed:
jsdoccomment-dist

I have tested this change with my consuming package.

@brettz9
Copy link
Contributor

brettz9 commented Mar 20, 2024

When I checkout your main branch, I am automatically getting unstaged modifications which lead to a different build of /docs/assets/dmt/icons/service/github.png and /docs/assets/dmt/icons/service/npm.png. Resetting the files doesn't help as they are immediately modified again. This holds true even after running pnpm i. Any idea what's going on?

image

@typhonrt
Copy link
Contributor Author

When I checkout your main branch, I am automatically getting unstaged modifications which lead to a different build of /docs/assets/dmt/icons/service/github.png and /docs/assets/dmt/icons/service/npm.png. Resetting the files doesn't help as they are immediately modified again. This holds true even after running pnpm i. Any idea what's going on?

I saw that too and it turns out that git will treat all files as text and it's a line ending CRLF / LF swap. I added to .gitattributes two more directives to treat jpg and png files as binary.

@brettz9 brettz9 merged commit dd6dac6 into es-joy:main Mar 20, 2024
9 checks passed
@brettz9
Copy link
Contributor

brettz9 commented Mar 20, 2024

Thanks for the PR. I added /docs to Pages. Anything else before I bump and release?

@typhonrt
Copy link
Contributor Author

Thanks for the PR. I added /docs to Pages. Anything else before I bump and release?

Everything looks good to me.

I'm glad to have committed the main previous PR effort for the larger effort of improving the AST manipulation capabilities to jsdoccomment so many dependent tooling efforts can benefit from that and thanks for trusting this PR / esm-d-ts and the tooling I put out.

I'll circle back around in a week or so when I have the final 0.3.0 esm-d-ts release published with a small PR bumping the version. I definitely do hope to get wider awareness in the dev community of this tooling circulating this year!

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