Repository navigation
feat: Add deno as a package manager - #905
Conversation
e0b6ff1 to
0c5a3bc
Compare
0c5a3bc to
27d7d76
Compare
s1gr1d
left a comment
There was a problem hiding this comment.
Thanks for adding this 🙌
I only added a small comment.
| detect: () => | ||
| ['deno.lock'].some((lockfile) => | ||
| fs.existsSync(path.join(process.cwd(), lockfile)), | ||
| ), |
There was a problem hiding this comment.
This could be surrounded with a try/catch
There was a problem hiding this comment.
It looks like only other package managers that have a try catch block are the yarn ones, would you like me to add them to others while I'm here?
There was a problem hiding this comment.
Yes, that would be awesome. Thanks!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #905 +/- ##
========================================
Coverage 51.79% 51.79%
========================================
Files 53 53
Lines 3657 3678 +21
Branches 854 872 +18
========================================
+ Hits 1894 1905 +11
+ Misses 1762 1642 -120
- Partials 1 131 +130
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@joshdales The tests are failing as the E2E tests are now using deno as default as it seems 🤔 |
|
@s1gr1d Looks like it's failing because I added the Deno option after Bun and the test just goes down one option: https://github.com/getsentry/sentry-wizard/blob/master/e2e-tests/tests/nuxt-3.test.ts#L51 so now it's picking Deno instead of Yarn 😅. I'll update these tests tonight, and double check that it doesn't happen in any other tests |
|
🤔 Looks the test did all the installation correctly, so that's progress, they then timed out at the build, I ran the e2e suite locally and the same thing happened on the master branch as well |
| }; | ||
|
|
||
| export const packageManagers = [BUN, YARN_V1, YARN_V2, PNPM, NPM]; | ||
| export const packageManagers = [BUN, DENO, YARN_V1, YARN_V2, PNPM, NPM]; |
There was a problem hiding this comment.
Can you maybe change the ordering here. We just discussed this internally and think that ordering by popularity would make more sense here.
| export const packageManagers = [BUN, DENO, YARN_V1, YARN_V2, PNPM, NPM]; | |
| export const packageManagers = [NPM, YARN_V1, YARN_V2, PNPM, BUN, DENO]; |
|
Hm, it's weird that the tests are failing here as the latest runs on master don't fail. Most probably, it's related to this PR (but could be there's another issue). |
|
Yeah, it's weird, as there is nothing obvious that would explain why the build step timesout. I saw that there were a couple of dependabot PRs that failed at the same stage in the nuxt ones as well, so could be something more? Regardless I'll update to try and minimise the changes. |
|
Will merge this now, thanks so much @joshdales for contributing this 🎉 |
deno as a package manager
Adds deno as a package manager, so that it is automatically detected. Resolves #894.
I built the package locally and ran it with
npxin a sveltekit application that uses Deno, it automatically detected that i was using deno and installed it 🎉.Full output from that is here: