Skip to content

Conversation

@JulianFlesch
Copy link
Contributor

Closes #3952

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@ningyuxin1999 ningyuxin1999 self-assigned this Dec 2, 2025
@JulianFlesch JulianFlesch changed the title switch pre-commit to prek for development Implement wave container commands Dec 2, 2025
Comment on lines +17 to +19
IMAGE_KEY = "name"
BUILD_ID_KEY = "buildId"
SCAN_ID_KEY = "scanId"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think any of these will change in the future. clearer if we just use them as strings in the code I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you don't feel strongly about this, I think we should keep them. They only exist in the class scope, close to where they are being used, but there we use them quite a lot as it helps us to not have redundant literals

for platform in CONTAINER_PLATFORMS:
containers[cs][platform] = self.request_container(cs, platform, env_path, await_, dry_run)

for platform in CONTAINER_PLATFORMS:
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to loop twice over container_platforms?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it makes it more readable what's being done with being less nested and clearly separated. But for comparing, applied your suggestion in af97d90

@JulianFlesch
Copy link
Contributor Author

JulianFlesch commented Dec 9, 2025

  • Test still missing ( @ningyuxin1999 )
  • Linting still missing
  • Sorting the resulting meta.yml after running create() still missing
  • Refactoring ModuleInfo to be helpful here -> TODO: ISSUE
  • Decision required: Behavior if modules don't have meta.yml and/or environment.yml

fut = pool.submit(self.request_container, cs, platform, self.condafile, await_)
tasks[fut] = (cs, platform)

for fut in as_completed(tasks):
Copy link
Contributor

Choose a reason for hiding this comment

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

why fut?

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.

4 participants