Skip to content

simplify loop in collect_exts_file_info#4689

Open
Flamefire wants to merge 4 commits intoeasybuilders:developfrom
Flamefire:exts-file-info-simplify
Open

simplify loop in collect_exts_file_info#4689
Flamefire wants to merge 4 commits intoeasybuilders:developfrom
Flamefire:exts-file-info-simplify

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

The loop over exts_list uses an excessive amount of branches and hence indentation making it hard to read.

We have 3 simple cases:

  1. Extension is a string -> use it as the name and do nothing else
  2. extension is neither a string, list nor dict -> Error out
  3. extension is a list of only a name -> use as the name and do nothing else

Moving the type check to the top of the loop makes it instantly visible which types are accepted, the elifs at the bottom are hard to find after all the indents.

Using continue after handling the simple cases reduces the indentation by 2 levels and makes it clear, that nothing else is done for those.

@Flamefire Flamefire force-pushed the exts-file-info-simplify branch from b3a7782 to 640b3ea Compare October 23, 2024 14:21
@boegel boegel added this to the 5.0 milestone Nov 6, 2024
@boegel boegel modified the milestones: 5.0.0, 5.x Mar 17, 2025
@boegel boegel changed the base branch from 5.0.x to develop March 19, 2025 10:51
@boegel boegel modified the milestones: 4.x, 5.x Mar 19, 2025
@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 19, 2025

@Flamefire I changed to target branch in this PR from 5.0.x to develop, you should synchronize your PR branch with current develop branch (which has received a massive update after the release of EasyBuild v5.0.0, see #4825)

@Flamefire Flamefire force-pushed the exts-file-info-simplify branch from 640b3ea to 057f268 Compare March 19, 2025 11:56
@boegel boegel modified the milestones: 5.x.x, 5.x Apr 9, 2025
@Flamefire Flamefire force-pushed the exts-file-info-simplify branch from 057f268 to 925909a Compare July 30, 2025 08:13
@Flamefire
Copy link
Copy Markdown
Contributor Author

Rebased

@Flamefire Flamefire force-pushed the exts-file-info-simplify branch from 925909a to caef5c9 Compare October 7, 2025 12:33
@Flamefire Flamefire force-pushed the exts-file-info-simplify branch from caef5c9 to d8d1fc5 Compare December 1, 2025 12:39
@boegel
Copy link
Copy Markdown
Member

boegel commented Dec 3, 2025

@Flamefire CI isn't happy with this?

 FAILED (failures=6, errors=2, skipped=3)

@Flamefire Flamefire force-pushed the exts-file-info-simplify branch from d8d1fc5 to 19d35a5 Compare December 3, 2025 08:39
@Flamefire
Copy link
Copy Markdown
Contributor Author

Looks like I over-indented one line. Fixed with rebase amending the faulty commit

The loop body is so large, that the allowed/checked types are very hard
to find.
When we only have a name, just continue and avoid the extra indent
@Flamefire Flamefire force-pushed the exts-file-info-simplify branch from b0cb62e to 0a7b0b7 Compare March 27, 2026 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants