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

mise: manifest updated to more portable config #6453

Merged
merged 10 commits into from
Jan 17, 2025

Conversation

ev-dev
Copy link
Contributor

@ev-dev ev-dev commented Jan 13, 2025

Included manifest changes suggested by @chawyehsu in PR comment which sets the internal mise configurable env var MISE_DATA_DIR to a directory that scoop persists

Relates to #6374

  • removed install/uninstall scripts
  • set env var MISE_DATA_DIR to a directory inside scoop and persist it
  • add the directory mise places its own shims to the Path env var
  • set env var MISE_GLOBAL_CONFIG_FILE to file inside app install dir and persist it
  • added pre_install script to check mise config.toml exists or is created as a file
  • added note about configuring specific shells with link to mise docs

…nal env vars

Included manifest changes suggested by @chawyehsu in PR ScoopInstaller#6374 which sets the internal mise configurable env var MISE_DATA_DIR to a directory that scoop persists
@ev-dev ev-dev mentioned this pull request Jan 13, 2025
2 tasks
Copy link
Contributor

All changes look good.

Wait for review from human collaborators.

mise

  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate
  • Autoupdate Hash Extraction

@ev-dev ev-dev marked this pull request as draft January 13, 2025 17:10
@ev-dev

This comment was marked as resolved.

@ev-dev ev-dev marked this pull request as ready for review January 13, 2025 17:33
bucket/mise.json Outdated Show resolved Hide resolved
bucket/mise.json Outdated Show resolved Hide resolved
@ev-dev ev-dev requested a review from chawyehsu January 15, 2025 22:14
Copy link
Contributor

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

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

LGTM

@stevehipwell
Copy link
Contributor

@chawyehsu is there a reason you didn't merge this after approving it?

@chawyehsu chawyehsu merged commit 30ea341 into ScoopInstaller:master Jan 17, 2025
2 checks passed
@chawyehsu
Copy link
Member

@chawyehsu is there a reason you didn't merge this after approving it?

I just forgot to

@eg-bernardo
Copy link

With this change the config is lost on every update.

I think the issue is that it's using $dir where $persist_dir should be used

@eg-bernardo
Copy link

I was wrong, because config.toml is in persist key of manifest.
This change still had the issue that it did not migrate the config from the previous location, so the config was lost on the update to this version, but future updates should not lose the config.

@ev-dev
Copy link
Contributor Author

ev-dev commented Jan 24, 2025

Yea good point @eg-bernardo really wish I hadn't forgotten to include that before the manifest went live in the main bucket 🤦 #6478

It may be best to submit a quick patch that just prompts new user installs with a warning message about manually migrating any previous configs/data.

@ev-dev ev-dev deleted the patch-1 branch January 24, 2025 05:32
@ev-dev
Copy link
Contributor Author

ev-dev commented Jan 24, 2025

#6484 adds warning message for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants