-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fixes for micromamba installs #24
Conversation
WalkthroughThe pull request includes updates to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
CHANGELOG.md (1)
12-14
: LGTM! Consider minor formatting adjustment.The fixes for
micromamba
behaviors are well-documented and align with the PR objectives. These changes should help address the reported installation issues.For consistency with the Keep a Changelog format, consider adjusting the indentation of the list items:
- Fix for new odd `micromamba` behaviors - - Set `MAMBA_ROOT_PREFIX` - - Set `use_lockfiles: False` in `.mambarc` + - Set `MAMBA_ROOT_PREFIX` + - Set `use_lockfiles: False` in `.mambarc`🧰 Tools
🪛 LanguageTool
[duplication] ~13-~13: Possible typo: you repeated a word
Context: ... for new oddmicromamba
behaviors - SetMAMBA_ROOT_PREFIX
- Setuse_lockfiles: False
in.mambarc
#...(ENGLISH_WORD_REPEAT_RULE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- install_miniforge.bash (4 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[duplication] ~13-~13: Possible typo: you repeated a word
Context: ... for new oddmicromamba
behaviors - SetMAMBA_ROOT_PREFIX
- Setuse_lockfiles: False
in.mambarc
#...(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (7)
CHANGELOG.md (2)
Line range hint
20-26
: LGTM! Empty sections maintained for clarity.The inclusion of empty "Added", "Removed", and "Deprecated" sections is a good practice. It maintains the structure of the changelog and clearly indicates that no changes were made in these categories.
🧰 Tools
🪛 LanguageTool
[duplication] ~13-~13: Possible typo: you repeated a word
Context: ... for new oddmicromamba
behaviors - SetMAMBA_ROOT_PREFIX
- Setuse_lockfiles: False
in.mambarc
#...(ENGLISH_WORD_REPEAT_RULE)
Line range hint
1-26
: Overall changes align well with PR objectives.The changelog updates effectively document the fixes for
micromamba
behaviors and the version update, which align well with the PR objectives of addressing GEOSpyD installation issues. The changes are clear and provide good context for the modifications made in this pull request.🧰 Tools
🪛 LanguageTool
[duplication] ~13-~13: Possible typo: you repeated a word
Context: ... for new oddmicromamba
behaviors - SetMAMBA_ROOT_PREFIX
- Setuse_lockfiles: False
in.mambarc
#...(ENGLISH_WORD_REPEAT_RULE)
install_miniforge.bash (5)
76-76
: LGTM: Version update for EXAMPLE_MINI_VERSIONThe update from "24.7.1-0" to "24.7.1-2" for the EXAMPLE_MINI_VERSION variable is appropriate. This minor version update likely includes bug fixes or small improvements, which aligns with the PR objective to address recent installation issues.
431-431
: LGTM: Added use_lockfiles configuration to .condarcThe addition of
use_lockfiles: False
to the .condarc configuration is consistent with the change made to .mambarc. This ensures that both mamba and conda configurations are aligned, which is a good practice for maintaining consistency in the environment management.
Line range hint
1-867
: Summary: Improvements to Miniforge installation processThe changes made to
install_miniforge.bash
are well-aligned with the PR objectives to address recent issues with GEOSpyD installations. The updates include:
- Updating the Miniforge version
- Disabling lock files for both mamba and conda
- Setting the MAMBA_ROOT_PREFIX
These modifications should help improve the installation process and resolve some of the reported issues. The changes are consistent and well-implemented throughout the script.
421-421
: LGTM: Added use_lockfiles configuration to .mambarcThe addition of
use_lockfiles: False
to the .mambarc configuration is appropriate. This change can help speed up installations, which may address some of the reported issues. However, be aware that disabling lock files might reduce consistency across different environments.To ensure this change doesn't introduce unexpected behavior, consider running the following verification script:
443-444
: LGTM: Added MAMBA_ROOT_PREFIX exportThe addition of
export MAMBA_ROOT_PREFIX=$MINIFORGE_INSTALLDIR
is a good practice. This ensures that mamba uses the correct installation directory, which can help prevent issues related to conflicting installations or incorrect path references. This change aligns well with the PR objectives to address installation issues.To ensure this change is effective, consider running the following verification script:
Recently, there have been weird issues seen with GEOSpyD installs by @JulesKouatchou.
Setting
MAMBA_ROOT_PREFIX
seems to help, but testing is ongoing.