Skip to content

Conversation

@lexming
Copy link
Contributor

@lexming lexming commented Apr 25, 2025

Follow-up to #4858 with code cleanup.

The logic in EasyBlock.expand_module_search_path is very much independent of the EasyBlock itself, only needing the path to the installation directory. Therefore it is simpler to move that code into ModuleEnvironmentVariable, where the attributes of the environment variable are known (e.g. its type) and closer to the rest of the code handling them.

@boegel boegel changed the title move EasyBlock.expand_module_search_path into ModuleEnvironmentVariable.expand_paths move EasyBlock.expand_module_search_path into ModuleEnvironmentVariable.expand_paths May 7, 2025
@boegel boegel added this to the release after 5.0.1 milestone May 7, 2025
@lexming lexming force-pushed the modenvvar-expand-paths branch from 2631db1 to 8f0135a Compare May 13, 2025 08:10
…d by EasyBlock.module_load_environment.expand_paths
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm, went through this in detail together with @lexming, carefully checking whether ModuleEnvironmentVariable.expand_paths method is equivalent to the code that was there in EasyBlock.expand_module_search_path)

mark `EasyBlock.expand_module_search_path` as removed, has been replaced by `EasyBlock.module_load_environment.expand_paths`
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel merged commit 0e469b0 into easybuilders:develop May 21, 2025
37 checks passed
@lexming lexming deleted the modenvvar-expand-paths branch May 21, 2025 16:03
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.

2 participants