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

fix: migration cleanup #10195

Merged
merged 13 commits into from
Jun 21, 2023
Merged

fix: migration cleanup #10195

merged 13 commits into from
Jun 21, 2023

Conversation

dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented Jun 20, 2023

  • remove obsolete |local
  • migration guide link
  • handle jsdoc
  • prompt transition migration
  • log what is migrated
  • bump versions inside package.json
  • folder prompt

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

@changeset-bot
Copy link

changeset-bot bot commented Jun 20, 2023

🦋 Changeset detected

Latest commit: 16c5057

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

This PR includes changesets to release 1 package
Name Type
svelte-migrate Patch

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

@dummdidumm dummdidumm changed the title Migration cleanup fix: igration cleanup Jun 20, 2023
@dummdidumm dummdidumm changed the title fix: igration cleanup fix: migration cleanup Jun 20, 2023
@@ -10,7 +10,7 @@ export async function migrate() {
bail('Please re-run this script in a directory with a package.json');
}

console.log(colors.bold().yellow('\nThis will update files in the current directory\n'));
console.log(colors.bold().yellow('\nThis will update files in the current src/ directory\n'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the path configurable?

Outside of Kit projects I've usually seen people put svelte files in client, app, components...

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 isn't right now. How could we go about it instead? Scan everything except node_modules / .svelte-kit or all things inside .gitignore? If a jsconfig/tsconfig exists and read the paths from there?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably keep it simple and add a clack prompt. The automated solutions would still miss things, whereas people can just order it to do the right thing for their codebase with an input.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, there is already a prompt for the transitions change. Great.

Copy link
Contributor

@gtm-nayan gtm-nayan left a comment

Choose a reason for hiding this comment

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

Maybe we should also add a note about running it individually inside monorepos? I think it might traverse some unwanted files if ran directly from the workspace.

packages/migrate/migrations/svelte-4/index.js Show resolved Hide resolved
packages/migrate/migrations/svelte-4/index.js Show resolved Hide resolved
packages/migrate/migrations/svelte-4/migrate.js Outdated Show resolved Hide resolved
Co-authored-by: gtmnayan <50981692+gtm-nayan@users.noreply.github.com>
Co-authored-by: gtmnayan <50981692+gtm-nayan@users.noreply.github.com>
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