Add support for multiple init scripts#2578
Add support for multiple init scripts#2578sfodje wants to merge 3 commits intotestcontainers:masterfrom
Conversation
This is in response to issue testcontainers#2232
|
|
||
| public SELF withInitScript(String initScriptPath) { | ||
| this.initScriptPath = initScriptPath; | ||
| public SELF withInitScript(String... initScriptPath) { |
There was a problem hiding this comment.
this breaks binary compatibility. Please make sure that the old method (withInitScript(String)) works as before
Added separate multiInitScript methods
| return withMultiInitScript(initScriptPath); | ||
| } | ||
|
|
||
| public SELF withMultiInitScript(String... initScriptPaths) { |
There was a problem hiding this comment.
Just a thought but... If the parameters were String, String... couldn't we keep the original method name and avoid problems with overloading/binary incompatibility? 🤔
There was a problem hiding this comment.
I had the same thought, but assumed the purpose was to keep the API visually and functionally the same. Now that you raise it up, it is a good question. The previous iteration was a lot simpler and less complicated.
| * @param initScriptPaths the resources to load the init scripts from | ||
| */ | ||
| public static void runInitScript(DatabaseDelegate databaseDelegate, String initScriptPath) { | ||
| public static void runMultiInitScript(DatabaseDelegate databaseDelegate, String... initScriptPaths) { |
There was a problem hiding this comment.
Do we need such big changes to ScriptUtils for this? It looks like this other PR has a simpler approach, and I can't see why that wouldn't work...
|
Hello @rnorth / @bsideup /@kiview, |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this. |
|
This issue has been automatically closed due to inactivity. We apologise if this is still an active problem for you, and would ask you to re-open the issue if this is the case. |
Inspired by PRs: testcontainers#2578 and testcontainers#1997 Closes testcontainers#2232
Inspired by PRs: testcontainers#2578 and testcontainers#1997 Closes testcontainers#2232
Inspired by PRs: testcontainers#2578 and testcontainers#1997 Closes testcontainers#2232
5 participants
This is in response to issue #2232