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.

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

feat: add meilisearch-module #2835

Merged
merged 27 commits into from
Oct 21, 2024

Conversation

mdelapenya
Copy link
Member

@mdelapenya mdelapenya commented Oct 18, 2024

What does this PR do?

Add Meilisearch Golang module

It takes all the commits in #2823, adding the commits needed to add the module to the docs system and the CI.

Why is it important?

Meilisearch is getting more popular and there Is already a java module for it.

Related issues

@mdelapenya mdelapenya requested a review from a team as a code owner October 18, 2024 12:15
@mdelapenya mdelapenya added the enhancement New feature or request label Oct 18, 2024
@mdelapenya mdelapenya self-assigned this Oct 18, 2024
Copy link

netlify bot commented Oct 18, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 711c06a
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/6716304d2a5cef0008cd5a6e
😎 Deploy Preview https://deploy-preview-2835--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@stevenh stevenh left a comment

Choose a reason for hiding this comment

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

Not sure you wanted the description you have currently.

modules/meilisearch/meilisearch.go Outdated Show resolved Hide resolved
modules/meilisearch/meilisearch.go Outdated Show resolved Hide resolved
modules/meilisearch/meilisearch.go Outdated Show resolved Hide resolved
modules/meilisearch/meilisearch_test.go Outdated Show resolved Hide resolved
modules/meilisearch/meilisearch_test.go Outdated Show resolved Hide resolved
modules/meilisearch/meilisearch_test.go Outdated Show resolved Hide resolved
modules/meilisearch/meilisearch_test.go Outdated Show resolved Hide resolved
modules/meilisearch/meilisearch_test.go Outdated Show resolved Hide resolved
modules/meilisearch/meilisearch_test.go Outdated Show resolved Hide resolved
modules/meilisearch/examples_test.go Outdated Show resolved Hide resolved
@mdelapenya mdelapenya requested a review from stevenh October 18, 2024 15:36
@mdelapenya
Copy link
Member Author

@stevenh I addressed all your comments 🤞

@stevenh stevenh mentioned this pull request Oct 18, 2024
@mashail
Copy link
Contributor

mashail commented Oct 20, 2024

@mdelapenya thank you for the help and the support.

LGTM @mdelapenya @stevenh

Copy link
Collaborator

@stevenh stevenh left a comment

Choose a reason for hiding this comment

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

Few nits, but looks like change to remove the unnecessary t.Run from the last review was missed?

modules/meilisearch/meilisearch_test.go Outdated Show resolved Hide resolved
modules/meilisearch/meilisearch_test.go Outdated Show resolved Hide resolved
testcontainers.CleanupContainer(t, ctr)
require.NoError(t, err)

t.Run("Connect to Address", func(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this was missed?

modules/meilisearch/meilisearch_test.go Outdated Show resolved Hide resolved
@mdelapenya
Copy link
Member Author

@stevenh everything was addressed. I missed the second t.Run, which was removed in 51a4c82

@mdelapenya mdelapenya requested a review from stevenh October 21, 2024 10:56
@mdelapenya mdelapenya merged commit 2c66f20 into testcontainers:main Oct 21, 2024
122 checks passed
@mdelapenya mdelapenya deleted the melisearch-module branch October 21, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants