Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

Feature/elasticsearch stopwords #5917

Merged
merged 15 commits into from
Nov 18, 2019
Merged

Feature/elasticsearch stopwords #5917

merged 15 commits into from
Nov 18, 2019

Conversation

luanalves
Copy link
Contributor

@luanalves luanalves commented Nov 2, 2019

Purpose of this pull request

Change elasticsearch stopwords directory to custom module

Affected DevDocs pages

https://devdocs.magento.com/guides/v2.3/config-guide/elasticsearch/es-config-stopwords.html

whatsnew
Added the "To change the directory from your module" section to the Configure Elasticsearch Stopwords in the Configuration Guide.

@devops-devdocs
Copy link
Collaborator

An admin must run tests on this PR before it can be merged.

</arguments>
</type>
```
3. In your module, create the directory `etc/stopwords`, inside add files .csv
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, change the order of the words in the following explanation.

Suggested change
3. In your module, create the directory `etc/stopwords`, inside add files .csv
3. In your module, create the directory `etc/stopwords`, with the corresponding `.csv` files inside.

Also, it would be really nice to have a reference (link) to the existing stopwords implementation in the Magento_Elasticsearch module.

@rogyar rogyar added 2.3.x Magento 2.3 related changes Technical Updates to the code or processes that alter the technical content of the doc labels Nov 2, 2019
@rogyar
Copy link
Contributor

rogyar commented Nov 2, 2019

Example from the codebase: https://github.com/magento/magento2/blob/7aa94564d85e408baea01abc5315a0441401c375/app/code/Magento/Elasticsearch/etc/di.xml#L332

@rogyar
Copy link
Contributor

rogyar commented Nov 2, 2019

@luanalves I would also kindly ask you to take a look at the failing tests. Thank you!

```

1. In your module, create the directory `etc/stopwords`, with the corresponding `.csv`.
[files inside.](https://github.com/magento/magento2/tree/2.3-develop/app/code/Magento/Elasticsearch/etc/stopwords){:target="_blank"}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, use the relative blob links instead of the direct absolute links. You can find an example at the very bottom of the following file.

I.e.

[lib/web/mage/accordion.js]: {{ site.mage2bloburl }}/{{ page.guide_version }}/lib/web/mage/accordion.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @rogyar the changes were taken care of but i had to add new variable

Copy link
Contributor

@dobooth dobooth left a comment

Choose a reason for hiding this comment

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

Hi @luanalves Thanks for the contribution. We are not sure why you are creating a new variable. Files like _config.yml are important and they support the system in many ways. We would rather you not edit it. If you can back out that change, that would be great. Please use the existing vars if you can, or a direct link if you need a specific path.

@luanalves
Copy link
Contributor Author

@dobooth unffortunally, in the file _config.yml doesn't exists the variable or value reference a information "https://github.com/magento/magento2/tree", it is necessary .

@dobooth
Copy link
Contributor

dobooth commented Nov 6, 2019

Hi @luanalves Please remove the new variable and use the blob path or a direct link to the tree if the distinction is critical.

@dobooth dobooth added Major Update Significant original updates to existing content Progress: changes requested and removed Progress: approved Technical Updates to the code or processes that alter the technical content of the doc labels Nov 18, 2019
@dobooth
Copy link
Contributor

dobooth commented Nov 18, 2019

running tests

@dobooth
Copy link
Contributor

dobooth commented Nov 18, 2019

running tests

@dobooth dobooth merged commit 15313ca into magento:master Nov 18, 2019
@ghost
Copy link

ghost commented Nov 18, 2019

Hi @luanalves, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2.3.x Magento 2.3 related changes Major Update Significant original updates to existing content Partner: Webjump partners-contribution PR created by Magento partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants