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

chore: move build to tsup, update top level entries, ignore artifacts #62

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jarredkenny
Copy link

Updates

  • Moved from direct calls to esbuild to tsup based ESM and CJS builds
  • Updated top level package entries to reference new js and mjs files respectively
  • gitignored previously generated build artifacts

Copy link
Owner

Choose a reason for hiding this comment

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

This repo used to use yarn, but I've recently switched to pnpm. Not sure if there's still a yarn lock file sitting around somewhere that I missed.
If that's the case, my apologies. Let me know if you find it.

That said, can you remove this file from the PR.
Also if you want to add it the .gitignore, you could still use yarn locally and it won't be part of the PR.

Copy link
Owner

@lancetipton lancetipton Sep 12, 2023

Choose a reason for hiding this comment

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

I'm not sure I follow why the /docs files were removed?
They are needed for the demo website => https://lancetipton.github.io/parkin/
But maybe I'm missing something?

I'd be open to removing them here and adding them to a github/pages branch only. Which is then used to pushed to the demo site. It doesn't take much to switch to that pattern, there's a few npm pacakges that help with it as well.

Copy link
Owner

@lancetipton lancetipton Sep 12, 2023

Choose a reason for hiding this comment

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

I have a custom publish cli / cmd that I run locally the uses the /docs and /build files.
I get why you ignored them, but means I probably need to change how new versions are published.

I'm completely open to that, and most likely it would be best to add it as a github action , so it's automated when changes are merged into main.

But until that's added I'd have to figure out a new way to publish the changes to github/pages for the demo site and npm for the package if these files are no longer part of the repo?

Or I'm open to another path if you have something in mind?

Copy link
Owner

@lancetipton lancetipton left a comment

Choose a reason for hiding this comment

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

wow, nice. Thank for the work you put into this. I'm not familiar with tsup, but I just looked it up and it looks solid.
I'm definitely open to switching to it. That said, there's a few issues I left comments for in this PR.

But, I'm happy to merge it once those are addressed.

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.

None yet

2 participants