Visitar URL original
Add interface to customize CreateContainerCmd by eddumelendez · Pull Request #7421 · testcontainers/testcontainers-java · GitHub
Skip to content

Add interface to customize CreateContainerCmd - #7421

Merged
eddumelendez merged 8 commits into
mainfrom
createcontainercmdcustomizer
Aug 21, 2023
Merged

eddumelendez merged 8 commits into
mainfrom
createcontainercmdcustomizer

Conversation

@eddumelendez

@eddumelendez eddumelendez commented Aug 17, 2023 •

Copy link
Copy Markdown
Member

Add CreateContainerCmdModifier interface to customize
CreateContainerCmd via SPI (global customization). Also,
withCreateContainerCmdModifier can be used too for local
customization.

Customization is applied after Testcontainers configuration is set.

@eddumelendez
eddumelendez requested a review from a team August 17, 2023 18:42
@eddumelendez eddumelendez added this to the next milestone Aug 17, 2023
Add `CreateContainerCmdCustomizer` interface to customize
`CreateContainerCmd`. Customization is applied after Testcontainers
configuration is set.
@eddumelendez
eddumelendez force-pushed the createcontainercmdcustomizer branch from 31293c2 to 3a02788 Compare August 17, 2023 19:04
@eddumelendez eddumelendez changed the title Add interface to customize CreateContainerCmd via SPI Add interface to customize CreateContainerCmd Aug 17, 2023
return self();
}

public SELF withCreateContainerCmdCustomizer(CreateContainerCmdCustomizer customizer) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should think about either deprecating withCreateContainerCmdModifier or adding an overload, to avoid increasing the API surface. Maybe, as a first step, we could keep withCreateContainerCmdModifier only but make it populate localCreateContainerCustomizers?

Also, if we name it CreateContainerCmdModifier, then we could align both APIs and not have to deprecate the old one (just thinking out loud...) - WDYT?

@eddumelendez eddumelendez Aug 18, 2023 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

looks like I forgot something when trying to overload withCreateContainerCmdModifier 🙈. Now, this is done. My only question is about the location where createContainerCmdCustomizers is being called because createContainerCmdModifiers is currently called before. I think the current implementation is ok and we should drop createContainerCmdModifiers references due to are not being used.

@eddumelendez
eddumelendez requested a review from bsideup August 18, 2023 23:11
@eddumelendez
eddumelendez requested a review from bsideup August 20, 2023 15:51
@eddumelendez
eddumelendez merged commit d9519f9 into main Aug 21, 2023
@eddumelendez
eddumelendez deleted the createcontainercmdcustomizer branch August 21, 2023 16:31
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