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

docs: update cookbook #3718

Merged
merged 6 commits into from
Dec 19, 2024
Merged

docs: update cookbook #3718

merged 6 commits into from
Dec 19, 2024

Conversation

hverlin
Copy link
Contributor

@hverlin hverlin commented Dec 19, 2024

@hverlin
Copy link
Contributor Author

hverlin commented Dec 19, 2024

CC @syhol in case you have other suggestions. Otherwise, we can make create additional PRs maybe

@hverlin hverlin marked this pull request as ready for review December 19, 2024 19:29
Copy link
Contributor

@syhol syhol left a comment

Choose a reason for hiding this comment

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

Some notes but looks good

'''
```

## Example with `pnpm`
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I don't use camelCase in my task names. You could use either:

  • [tasks."pnpm:install"]
  • [tasks.pnpm-install]

I don't feel strongly about it though, so just a suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree that it will be nicer. I will update to pnpm-install

Comment on lines +54 to +57
root@75f179a190a1:/mise# echo "" >/mise/config.toml
root@75f179a190a1:/mise# mise prune --yes
mise pruned configuration links
mise python@3.13.1 ✓ remove /mise/cache/python/3.13.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth putting a little comment above this, mentioning that we overwrite and prune because jdx/mise docker image comes with python installed.

root@75f179a190a1:/mise# # overwrite config to give us a clean state

node = '22'

[hooks]
post_install = 'corepack enable'
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I think directly installing pnpm with mise is a nicer experience, but we have had a few people ask how to use corepack with mise.

Also corepack won't be built into newer versions of nodejs by default. It's worth thinking a bit more about this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I could show both. I like corepack more since it's installed the exact version which is in the package.json (pnpm will do it in v10, but this avoids installing pnpm twice)

I can replace it with npx corepack enable to future-proof it 👍

@jdx jdx merged commit 4df4933 into jdx:main Dec 19, 2024
21 checks passed
miguelmig pushed a commit to miguelmig/mise that referenced this pull request Dec 21, 2024
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.

3 participants