-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
BREAKING CHANGE: Clean up files in repo #2963
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
8782b08
to
5d36c43
Compare
We actually don't need |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks ok to me. Keeping in mind I'm not as familiar with all of the packaging steps, but this change makes sense to me.
@@ -24,6 +25,8 @@ async function init() { | |||
console.log('Cleaning up output directory ' + outputDir); | |||
await rm(outputDir, { force: true, recursive: true }); | |||
await mkdir(outputDir); | |||
console.log(`Copying file ${join(cwd, 'LICENSE.md')}`); | |||
console.log(` to ${join(inputDir, 'LICENSE.md')}`); | |||
await copyFile(join(cwd, 'LICENSE.md'), join(inputDir, 'LICENSE.md')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add something like:
await copyFile(join(cwd, 'marked.min.js'), join(outputDir, 'marked.min.js''));
Then in the client side JS of the Demo, you can fetch the latest for the given deployment with:
fetch(new URL('/marked.min.js', window.location))`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to get the PR vercel link with just a PR#?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found it based on the vercel comment View Preview link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or we don't even need the pr version because now we can go to the preview and use /marked.min.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, we don’t need a text input anymore. Either you select an old version or you select “HEAD”. And HEAD for a production deployment is master branch and for a preview deployment it’s the PR branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thanks! 🎉
# [9.0.0](v8.0.1...v9.0.0) (2023-09-09) ### Bug Fixes * Clean up files in repo ([#2963](#2963)) ([7d95a91](7d95a91)) ### BREAKING CHANGES * - remove built files from git repo. - If you need to use the latest version of marked on the web you can use a cdn to get marked.min.js from npm: - `https://cdn.jsdelivr.net/npm/marked/marked.min.js`
Marked version: 8.0.0
Description
Clean up files that are not needed in repo. I left
marked.min.js
since we need it in the repo to run our demo on master and specific commits and prs.This is only breaking if someone uses the files in lib/ on the master branch. Technically they were never meant to be used so I'm not sure we actually need to bump the major version.
Contributor
Committer
In most cases, this should be a different person than the contributor.