search

AngelaE / ts-mountebank

Mountebank client for TS / node
15 stars 5 forks source link

Migrate from `npm` to `pnpm` #27

Closed yeongsheng-tan closed 8 months ago

yeongsheng-tan commented 10 months ago

Motivation: Why you should prefer using pnpm over npm and yarn

AngelaE commented 10 months ago

Thanks for this! Do you plan to push more changes? I do not mind if you rebase your changes on the base branch and force push to tidy things up. I am really busy at the moment, that's why I am slow with the review.

It definitely makes sense to change to pnpm, but when I tried everything locally the integration tests failed because MB seems to be started after the tests. (I am on windows). Would be nice to fix that, but I prefer running MB extra anyway working locally - otherwise we would need to tear it down after each test run. Sometimes I like to look at the recorded requests etc.

yeongsheng-tan commented 10 months ago

Thanks for this! Do you plan to push more changes? I do not mind if you rebase your changes on the base branch and force push to tidy things up. I am really busy at the moment, that's why I am slow with the review.

It definitely makes sense to change to pnpm, but when I tried everything locally the integration tests failed because MB seems to be started after the tests. (I am on windows). Would be nice to fix that, but I prefer running MB extra anyway working locally - otherwise we would need to tear it down after each test run. Sometimes I like to look at the recorded requests etc.

Hi @AngelaE thanks for the feedback. I have perhaps 1 more change to upgrade chai to 4.4.0.

Noted to rebase my changes on the base branch and force push.

Let me try another way to fix mb starting slower than when integration-test kicks in.

yeongsheng-tan commented 10 months ago

If the above are alright and fixes your local integration tests, then I'll be happy to squash the commits and do a force push to clean up the history. Thanks @AngelaE

yeongsheng-tan commented 9 months ago

If the above are alright and fixes your local integration tests, then I'll be happy to squash the commits and do a force push to clean up the history. Thanks @AngelaE

I've squahed as much of the commits as I deemed makes sense and forced pushed to this PR branch.

Very much appreciate your review and merge soon. Thank you. 😉

yeongsheng-tan commented 8 months ago

@AngelaE any chance you would like to have this reveiwed and merged? Wonder if there is still any motivation to move this along.

AngelaE commented 8 months ago

@AngelaE any chance you would like to have this reveiwed and merged? Wonder if there is still any motivation to move this along.

I will try to merge it this weekend, apologies for not coming back to it. My life has been a bit mad. Am definitely interested in changing to pnpm!

AngelaE commented 8 months ago

Thanks, everything is running much faster now! Definitely notice this in the gh actions! Apologies for taking so long! I found the scripts confusing, which is why I put it aside around xmas. And then life became very stressful and I forgot 🙏

Waiting for the MB process during the test run is neat and works very well (now that I started the correct script)!

I re-arranged some scripts now and added a commit hook to fail if the project is still referenced in the integration-tests, see https://github.com/AngelaE/ts-mountebank/pull/29 if you are interested. Nothing big, mostly re-names and I pushed some responsibilities down to the integration-test project.

yeongsheng-tan commented 8 months ago

Thanks, everything is running much faster now! Definitely notice this in the gh actions! Apologies for taking so long! I found the scripts confusing, which is why I put it aside around xmas. And then life became very stressful and I forgot 🙏

Waiting for the MB process during the test run is neat and works very well (now that I started the correct script)!

I re-arranged some scripts now and added a commit hook to fail if the project is still referenced in the integration-tests, see #29 if you are interested. Nothing big, mostly re-names and I pushed some responsibilities down to the integration-test project.

Thank you so very much. Nice to see the tidy-up and renaming of package scripts.