-
Notifications
You must be signed in to change notification settings - Fork 9.4k
[ReadMe] updated readMe file for ImportExport-LayeredNavigation modules #31809
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
[ReadMe] updated readMe file for ImportExport-LayeredNavigation modules #31809
Conversation
Hi @vlmed. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
You can find more information about the builds here ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review. For more details, please, review the Magento Contributor Guide documentation. 🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket. 🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
06987ed
to
f7b6043
Compare
f7b6043
to
496d9ff
Compare
@magento run all tests |
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.
@vlmed great job, you covered a lot of information here! I added some change requests, but it's clear that you put a lot of work in this already - thank you!
|
||
## Installation | ||
|
||
The Magento_ImportExport module creates the following tables in the database: |
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.
Please add some information about the schema/column not being persistent (e.g. not having to be manually removed) when the module gets disabled and setup:upgrade is run.
|
||
### Public APIs | ||
|
||
- `Magento\ImportExport\Api\Data\ExportInfoInterface` |
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.
The ExtendedExportInfoInterface
interface is missing from the description. Please add it and describe in a little more detail both data interfaces so the readme provides clear enough info on what's the difference between them without having to look at the actual code.
- export data | ||
|
||
- `\Magento\ImportExport\Api\ExportManagementInterface`: | ||
- get export data |
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 is a bit misleading. "Export data" above and "get export data" here might suggest that this interface provides a repository-like getter, while in fact it provides logic for executing the actual export.
app/code/Magento/Indexer/README.md
Outdated
Magento_Indexer module is a base of Magento Indexing functionality. | ||
# Magento_Indexer module | ||
|
||
This module provides Magento Indexing functionality. | ||
It allows: |
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.
Please improve wording on this. Like:
- "It allows to:" and the rest as is
- "It allows:" and the rest in present continuous (reading, representing, regenerating, viewing)
app/code/Magento/Indexer/README.md
Outdated
|
||
- `clean_cache_by_tags` event in the `\Magento\Indexer\Model\Indexer\CacheCleaner::cleanCache` method. Parameters: | ||
- `object` is a `$this->cacheContext` object (`Magento\Framework\Indexer\CacheContext` class) |
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.
Imho this is a bit too detailed and dependent on the implementation. It's good to describe what class it is, but a description that this is the current cache context (instead of using an actual property name of the $this
object) would suffice.
|
||
### Layouts | ||
|
||
This module introduces the following layouts in the `view/frontend/layout` directory: |
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.
Please change wording from layouts to layout handles: https://devdocs.magento.com/guides/v2.4/frontend-dev-guide/layouts/layout-overview.html#layout-over-terms
Additionally, please include some basic info about how the module modifies layouts (things happening in the view/frontend/page_layout
directory), but that's actually more suited for Additional Information as this section is about extension points.
|
||
### Layouts | ||
|
||
This module introduces the following layouts in the `view/adminhtml/layout` directory: |
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.
Please change wording from layouts to layout handles: https://devdocs.magento.com/guides/v2.4/frontend-dev-guide/layouts/layout-overview.html#layout-over-terms
app/code/Magento/Indexer/README.md
Outdated
### Layouts | ||
|
||
This module introduces the following layouts in the `view/adminhtml/layout` directory: |
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.
Please change wording from layouts to layout handles: https://devdocs.magento.com/guides/v2.4/frontend-dev-guide/layouts/layout-overview.html#layout-over-terms
|
||
### Layouts | ||
|
||
This module introduces the following layouts in the `view/frontend/layout` directory: |
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.
Please change wording from layouts to layout handles: https://devdocs.magento.com/guides/v2.4/frontend-dev-guide/layouts/layout-overview.html#layout-over-terms
|
||
## Additional information | ||
|
||
More information can get at articles: |
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.
Please change the wording of this one to "More information can be found in:" or something similar to make this one grammatically correct
496d9ff
to
7f1dd3f
Compare
@magento run all tests |
@bgorski Thank you for the review. I improved PR according to your recommendation |
Hi @bgorski, thank you for the review. |
@magento run Functional Tests EE |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
1 similar comment
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
@magento run Functional Tests EE |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
Hi @Den4ik, thank you for the review. |
Hi @vlmed, thank you for your contribution! |
Description (*)
Update README.md file for modules:
Fixed Issues (if relevant)
Contribution checklist (*)