Skip to content

feat: switch to pnpm & Remove ember-source as a peer dependency#99

Merged
villander merged 2 commits intovillander:masterfrom
aklkv:feat/pnpm
Oct 20, 2025
Merged

feat: switch to pnpm & Remove ember-source as a peer dependency#99
villander merged 2 commits intovillander:masterfrom
aklkv:feat/pnpm

Conversation

@aklkv
Copy link
Contributor

@aklkv aklkv commented Jun 9, 2025

@aklkv aklkv force-pushed the feat/pnpm branch 3 times, most recently from 4f7d974 to e3cb6d6 Compare June 9, 2025 02:41
@aklkv aklkv marked this pull request as ready for review June 9, 2025 03:00
Copy link
Owner

@villander villander left a comment

Choose a reason for hiding this comment

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

Thank you for the great work! There are some questions and minor changes before we move forward.

"@types/ember__routing": "^4.0.22",
"babel-plugin-ember-template-compilation": "^2.2.5",
"concurrently": "^8.2.2",
"ember-source": "^6.4.0",
Copy link
Owner

Choose a reason for hiding this comment

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

Isn't it necessary to keep covering old Ember versions as we were doing before?

Copy link
Contributor Author

@aklkv aklkv Jul 22, 2025

Choose a reason for hiding this comment

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

ember-try does this for us and in this case we are only using ember-source for types only

"@release-it-plugins/workspaces": "^4.2.0",
"release-it": "^17.5.0"
},
"packageManager": "pnpm@10.12.1+sha512.f0dda8580f0ee9481c5c79a1d927b9164f2c478e90992ad268bbb2465a736984391d6333d2c327913578b2804af33474ca554ba29c04a8b13060a717675ae3ac",
Copy link
Owner

Choose a reason for hiding this comment

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

Why is it needed? It does not seem to be the right thing to do with this long version. Why can't we simply use 10.12.1?

https://stackoverflow.com/questions/71747609/how-to-specify-packagemanager-in-package-json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

corepack adds a sha for integrity check

Comment on lines +39 to +55
@action goToChineseVersion() {
this.transitionTo({ queryParams: { lang: 'Chinese' } });
}

@action transitionToHome() {
this.transitionToExternal('home').then(() => {
var postController = this.controllerFor(this.routeName);
postController.set('transitionedToExternal', true);
});
}

@action replaceWithHome() {
this.replaceWithExternal('home').then(() => {
var postController = this.controllerFor(this.routeName);
postController.set('replacedWithExternal', true);
});
}
Copy link
Owner

Choose a reason for hiding this comment

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

Why are we moving it from route to controller?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

route actions are deprecated

@@ -10,12 +10,3 @@ npm-debug.log*
yarn-error.log
Copy link
Owner

Choose a reason for hiding this comment

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

could you please remove it as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is part of blueprint so I didn't remove it as it was not showing in the diff

@aklkv aklkv requested a review from villander August 10, 2025 01:50
Copy link
Owner

@villander villander left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@villander villander merged commit 544abe2 into villander:master Oct 20, 2025
12 checks passed
@villander villander added the bug Something isn't working label Oct 20, 2025
@villander villander changed the title feat: switch to pnpm feat: switch to pnpm & Remove ember-source as a peer dependency Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants