Skip to content

Conversation

zt4ff
Copy link
Contributor

@zt4ff zt4ff commented Oct 11, 2022

…RCH_URL

Pull Request

Related issue

Fixes #339

What does this PR do?

  • It replace MEILISEARCH_HOST by MEILISEARCH_URL in the docker config

PR checklist

Please check if your PR fulfils the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

@brunoocasali
Copy link
Member

Thanks a lot for your contribution @zt4ff :D


public func currentHost() -> String {
ProcessInfo.processInfo.environment["MEILISEARCH_HOST"] ?? "http://localhost:7700"
ProcessInfo.processInfo.environment["MEILISEARCH_URL"] ?? "http://localhost:7700"
Copy link
Member

Choose a reason for hiding this comment

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

I can't comment in other places, but can you also change the name of the host param? It now should also be url.

example:
client = try MeiliSearch(host: "http://localhost:7700", apiKey: "masterKey") become client = try MeiliSearch(url: "http://localhost:7700", apiKey: "masterKey")

also, in the code-samples.meilisearch.yml and over the README :)

Copy link
Member

Choose a reason for hiding this comment

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

Better thinking is not the best way to forward with that. We should deprecate the current param first. Then in another PR and another release version, remove it.

Copy link
Member

Choose a reason for hiding this comment

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

So, can you "deprecate" the host from MeiliSearch param to merge this PR?

Copy link
Member

Choose a reason for hiding this comment

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

Better thinking again, we can do that later. Thanks!

@brunoocasali brunoocasali added the enhancement New feature or request label Oct 14, 2022
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

Thanks again for your PR!

bors merge

@meili-bors
Copy link
Contributor

meili-bors bot commented Oct 14, 2022

@meili-bors meili-bors bot merged commit 5fe6a06 into meilisearch:main Oct 14, 2022
@brunoocasali
Copy link
Member

This message is sent automatically

Thanks again for contributing to Meilisearch ❤️
If you are participating in Hacktoberfest, and you would like to receive some gift from Meilisearch too, please complete this form.

@zt4ff
Copy link
Contributor Author

zt4ff commented Oct 14, 2022

Thanks @brunoocasali

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.

Replace MEILISEARCH_HOST by MEILISEARCH_URL in the docker config

2 participants