Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add dockerEnv target to customize environment passed to docker command #3257

Merged
merged 3 commits into from
Jan 20, 2025
Merged

Add dockerEnv target to customize environment passed to docker command #3257

merged 3 commits into from
Jan 20, 2025

Conversation

lolgab
Copy link
Member

@lolgab lolgab commented Jul 6, 2024 •
edited
Loading

Fixes #3132

Pull Request: #3257

@lolgab
Copy link
Member Author

lolgab commented Jul 6, 2024

Suggestions accepted for the dockerEnv name

@lolgab lolgab requested review from joan38 and lefou and removed request for joan38 July 6, 2024 12:27
@lolgab lolgab marked this pull request as ready for review July 6, 2024 12:27
lefou
lefou reviewed Jul 6, 2024
View reviewed changes
Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

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

Do I understand correctly, that this env is about the invocation of the docker command? Since we now have two targets containing some envs in the docker module, we should clarify how they differentiate.

@lolgab
Copy link
Member Author

lolgab commented Jul 7, 2024 •
edited
Loading

I already added some scaladoc to the new target. They also have links to two different pages in docker documentation

@lolgab lolgab requested a review from lefou July 9, 2024 12:51
lefou
lefou approved these changes Jan 20, 2025
View reviewed changes
Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

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

Looks good to me.

The dockerEnv name is a bit misleading, but mostly because docker has a build-time and a runtime. This module is about building docker images but is called just "Docker". I think it's consistent enough for now.

@lolgab lolgab merged commit f4fbabf into com-lihaoyi:main Jan 20, 2025
28 checks passed
@lolgab lolgab deleted the docker-env branch January 20, 2025 17:37
@lefou lefou added this to the 0.12.6 milestone Jan 20, 2025
gamlerhart pushed a commit to gamlerhart/mill that referenced this pull request Feb 9, 2025
@lolgab @lihaoyi @gamlerhart
Add dockerEnv target to customize environment passed to docker c…
b00fa8d 
…ommand (com-lihaoyi#3257)

Fixes com-lihaoyi#3132

Pull Request: com-lihaoyi#3257

---------

Co-authored-by: Li Haoyi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lefou lefou lefou approved these changes

@joan38 joan38 Awaiting requested review from joan38

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.12.6
Development

Successfully merging this pull request may close these issues.

docker.build should invalidate results when environment changes
3 participants
@lolgab @lefou @lihaoyi