Skip to content

Conversation

@risu729
Copy link
Contributor

@risu729 risu729 commented Nov 27, 2025

The previous implementation didn't support MISE_OVERRIDE_CONFIG_FILENAMES with custom directories.

Copilot AI review requested due to automatic review settings November 27, 2025 05:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the hook_env.rs module to derive the config_subdirs list dynamically from DEFAULT_CONFIG_FILENAMES instead of using a hardcoded array. This improves maintainability by deriving the subdirectories from a single source of truth.

  • Adds import for DEFAULT_CONFIG_FILENAMES from the config module
  • Replaces hardcoded config subdirectory array with dynamic derivation using string splitting and deduplication

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@risu729 risu729 marked this pull request as draft November 27, 2025 05:29
@risu729 risu729 marked this pull request as ready for review November 27, 2025 05:38
@risu729 risu729 requested a review from Copilot November 27, 2025 07:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +134 to +138
let config_subdirs = DEFAULT_CONFIG_FILENAMES
.iter()
.map(|f| f.rsplit_once("/").map(|(dir, _)| dir).unwrap_or(""))
.unique()
.collect::<Vec<_>>();
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

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

[nitpick] The logic doesn't handle glob patterns correctly. When DEFAULT_CONFIG_FILENAMES contains patterns like .config/mise/conf.d/*.toml, the extracted directory .config/mise/conf.d would be checked, but this doesn't account for the fact that the actual config files might be in subdirectories matching the glob pattern.

However, checking the parent directory should still catch modifications when new files are added to that directory, so this is likely acceptable. Consider adding a comment explaining this behavior for future maintainers.

Copilot uses AI. Check for mistakes.
@jdx jdx merged commit 9269754 into jdx:main Nov 27, 2025
32 checks passed
@risu729 risu729 deleted the hook-env-config-subdirs branch November 27, 2025 12:02
jdx pushed a commit that referenced this pull request Nov 27, 2025
### 📦 Registry

- enable symlink_bins for aws-sam by @risu729 in
[#7082](#7082)
- use cargo backend for tokei to support latest version by @risu729 in
[#7086](#7086)
- add SonarSource/sonar-scanner-cli by @kapitoshka438 in
[#7087](#7087)

### 🐛 Bug Fixes

- **(docs)** link gitlab backended tools in registry by @risu729 in
[#7078](#7078)

### 🚜 Refactor

- **(hook-env)** derive config_subdirs from config filenames by @risu729
in [#7080](#7080)

### New Contributors

- @kapitoshka438 made their first contribution in
[#7087](#7087)

## 📦 Aqua Registry Updates

#### New Packages (1)

- [`kiali/kiali`](https://github.com/kiali/kiali)

#### Updated Packages (3)

- [`XAMPPRocky/tokei`](https://github.com/XAMPPRocky/tokei)
- [`astral-sh/uv`](https://github.com/astral-sh/uv)
- [`wtfutil/wtf`](https://github.com/wtfutil/wtf)
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.

2 participants