-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
Updated Vault docs, uplifted Vault image version #3685
Updated Vault docs, uplifted Vault image version #3685
Conversation
- removed VaultClientTest and moved its contest to VaultContainerTest - uplifted Vault image version - added getVaultAddress method for usage convenience
I'm not a vault user but looks good at a first glance. |
} | ||
|
||
@Test | ||
public void readFirstSecretPathOverHttpApi() throws InterruptedException { | ||
// readWithHttpApi { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment isn't really necessary - we could have the codeinclude
tag in the docs just say: inside_block:readFirstSecretPathOverHttpApi
.
This way, we avoid having the method signature public void readFirstSecretPathOverHttpApi()
appear in the docs, which is a bit distracting.
} | ||
|
||
@Test | ||
public void readFirstSecretPathOverHttpApi() throws InterruptedException { | ||
// readWithHttpApi { | ||
public void readFirstSecretPathOverHttpApi() { | ||
given(). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution - a couple of minor changes but I'm grateful that you're helping to improve the docs.
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the docs improvements and the new getVaultAddress
. @RamazanYapparov is there a chance to rebase the branch, resolve the conflicts (for example, we already use assertj) and make the changes requested?
modules/vault/src/main/java/org/testcontainers/vault/VaultContainer.java
Outdated
Show resolved
Hide resolved
@RamazanYapparov thanks for this PR! few comments were addressed but now this is merged in |
VaultClientTest
class and moved its test toVaultContainerTest
getVaultAddress
method for usage convenienceThis PR is also related to #1158 issue