MC-5465: Adding documentation draft #3
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pdohogne-magento
requested review from
buskamuza,
jeff-matthews,
melnikovi and
paliarush
CONTRIBUTING.md
Outdated
| Contributions to the Magento codebase are done using the fork & pull model. | ||
| This contribution model has contributors maintaining their own copy of the forked codebase (which can easily be synced with the main copy). The forked repository is then used to submit a request to the base repository to “pull” a set of changes (hence the phrase “pull request”). | ||
|
|
||
| Contributions can take the form of new components/features, changes to existing features, tests, documentation (such as developer guides, user guides, examples, or specifications), bug fixes, optimizations or just good suggestions. |
There was a problem hiding this comment.
Suggested change
| Contributions can take the form of new components/features, changes to existing features, tests, documentation (such as developer guides, user guides, examples, or specifications), bug fixes, optimizations or just good suggestions. | |
| Contributions can take the form of new components/features, changes to existing features, tests, documentation (such as developer guides, user guides, examples, or specifications), bug fixes, optimizations, or just good suggestions. |
CONTRIBUTING.md
Outdated
|
|
||
| Contributions can take the form of new components/features, changes to existing features, tests, documentation (such as developer guides, user guides, examples, or specifications), bug fixes, optimizations or just good suggestions. | ||
|
|
||
| The Magento development team will review all issues and contributions submitted by the community of developers in first in, first out order. During the review we might require clarifications from the contributor. If there is no response from the contributor for two weeks, the issue is closed. |
There was a problem hiding this comment.
Suggested change
| The Magento development team will review all issues and contributions submitted by the community of developers in first in, first out order. During the review we might require clarifications from the contributor. If there is no response from the contributor for two weeks, the issue is closed. | |
| The Magento development team will review all issues and contributions submitted by the community of developers in first-in, first-out order. During the review we might require clarifications from the contributor. If there is no response from the contributor for two weeks, the issue is closed. |
CONTRIBUTING.md
Outdated
|
|
||
| ## Contribution requirements | ||
|
|
||
| 1. Contributions must adhere to [Magento coding standards](http://devdocs.magento.com/guides/v2.0/coding-standards/bk-coding-standards.html). |
There was a problem hiding this comment.
Suggested change
| 1. Contributions must adhere to [Magento coding standards](http://devdocs.magento.com/guides/v2.0/coding-standards/bk-coding-standards.html). | |
| 1. Contributions must adhere to [Magento coding standards](http://devdocs.magento.com/guides/v2.3/coding-standards/bk-coding-standards.html). |
CONTRIBUTING.md
Outdated
| 1. Contributions must adhere to [Magento coding standards](http://devdocs.magento.com/guides/v2.0/coding-standards/bk-coding-standards.html). | ||
| 2. Pull requests (PRs) must be accompanied by a complete and meaningful description. Comprehensive descriptions make it easier to understand the reasoning behind a change and reduce the amount of time required to get the PR merged. | ||
| 3. Commits must be accompanied by meaningful commit messages. | ||
| 4. PRs which include bug fixing must be accompanied with step-by-step descriptions of how to reproduce the issue (including the local composer version reported by `composer --version`). |
There was a problem hiding this comment.
Suggested change
| 4. PRs which include bug fixing must be accompanied with step-by-step descriptions of how to reproduce the issue (including the local composer version reported by `composer --version`). | |
| 4. PRs which include bug fixing must be accompanied by step-by-step instructions to reproduce the issue (including the local composer version reported by `composer --version`). |
CONTRIBUTING.md
Outdated
|
|
||
| ### Composer compatibility | ||
|
|
||
| Maintaining compatibility with the Composer versions listed in [composer.json](composer.json) is of particular note for this project. Due to the way Composer works with plugins, the version that is used when the plugin runs is the local `composer.phar` executable version (as reported by `composer --version`) and not the version installed in the project's `vendor` folder or `composer.lock` file. This means that in order to properly verify Composer compatibility, tests must be run against the local `composer.phar` executable, not just the installed `composer/composer` dependency. |
There was a problem hiding this comment.
Suggested change
| Maintaining compatibility with the Composer versions listed in [composer.json](composer.json) is of particular note for this project. Due to the way Composer works with plugins, the version that is used when the plugin runs is the local `composer.phar` executable version (as reported by `composer --version`) and not the version installed in the project's `vendor` folder or `composer.lock` file. This means that in order to properly verify Composer compatibility, tests must be run against the local `composer.phar` executable, not just the installed `composer/composer` dependency. | |
| Maintaining compatibility with the Composer versions listed in the [composer.json](composer.json) file is important for this project. Due to the way Composer works with plugins, the version that is used when the plugin runs is the local `composer.phar` executable version (as reported by `composer --version`) and not the version installed in the project's `vendor` folder or `composer.lock` file. This means that in order to properly verify Composer compatibility, tests must be run against the local `composer.phar` executable, not just the installed `composer/composer` dependency. |
docs/process_flows.md
Outdated
|
|
||
| ## `composer require/update magento/composer-root-update-plugin` | ||
|
|
||
| **Scenario:** The user wants to install or update the version of the `magento/composer-root-update-plugin` package in their Magento installation. They call `composer require/update magento/composer-root-update-plugin`. The plugin needs to update a copy of itself in the `<Magento>/var` directory, where it needs to exist in order to function during Web Setup Wizard operations. |
There was a problem hiding this comment.
Suggested change
| **Scenario:** The user wants to install or update the version of the `magento/composer-root-update-plugin` package in their Magento installation. They call `composer require/update magento/composer-root-update-plugin`. The plugin needs to update a copy of itself in the `<Magento>/var` directory, where it needs to exist in order to function during Web Setup Wizard operations. | |
| **Scenario:** The user wants to install or update the version of the `magento/composer-root-update-plugin` package in their Magento installation. They call `composer require/update magento/composer-root-update-plugin`. The plugin needs to update a copy of itself in the `<Magento>/var` directory, where it is required for Web Setup Wizard operations. |
docs/process_flows.md
Outdated
| 4. `PluginDefinition::packageUpdate()` calls [WebSetupWizardPluginInstaller::packageEvent()](../src/Magento/ComposerRootUpdatePlugin/Setup/WebSetupWizardPluginInstaller.php) | ||
| 5. `WebSetupWizardPluginInstaller::packageEvent()` checks the event to see if it was triggered by a change to the `magento/composer-root-update-plugin` package, and if so it calls `WebSetupWizardPluginInstaller::updateSetupWizardPlugin()` | ||
| 6. `WebSetupWizardPluginInstaller::updateSetupWizardPlugin()` checks the `<Magento>/var/vendor` directory for the `magento/composer-root-update-plugin` version installed there to see if it matches the version in the triggered event | ||
| 7. If the version doesn't match or `magento/composer-root-update-plugin` is absent in `<Magento>/var/vendor`, `WebSetupWizardPluginInstaller::updateSetupWizardPlugin()` installs the new version of `magento/composer-root-update-plugin` in a temporary directory then replaces `<Magento>/var/vendor` with the `vendor` directory from the temporary installation |
There was a problem hiding this comment.
Suggested change
| 7. If the version doesn't match or `magento/composer-root-update-plugin` is absent in `<Magento>/var/vendor`, `WebSetupWizardPluginInstaller::updateSetupWizardPlugin()` installs the new version of `magento/composer-root-update-plugin` in a temporary directory then replaces `<Magento>/var/vendor` with the `vendor` directory from the temporary installation | |
| 7. If the version does not match or `magento/composer-root-update-plugin` is absent in `<Magento>/var/vendor`, `WebSetupWizardPluginInstaller::updateSetupWizardPlugin()` installs the new version of `magento/composer-root-update-plugin` in a temporary directory, then replaces `<Magento>/var/vendor` with the `vendor` directory from the temporary installation |
docs/process_flows.md
Outdated
|
|
||
| ## Magento module-based `var` installation | ||
|
|
||
| **Scenario:** The user has called the `bin/magento setup:uninstall` command, which clears the `<Magento>/var` directory, then runs `bin/magento setup:install`. The plugin needs to reinstall itself in the `<Magento>/var` directory, where it needs to exist in order to function during Web Setup Wizard operations. |
There was a problem hiding this comment.
Suggested change
| **Scenario:** The user has called the `bin/magento setup:uninstall` command, which clears the `<Magento>/var` directory, then runs `bin/magento setup:install`. The plugin needs to reinstall itself in the `<Magento>/var` directory, where it needs to exist in order to function during Web Setup Wizard operations. | |
| **Scenario:** The user calls the `bin/magento setup:uninstall` command, which clears the `<Magento>/var` directory, then runs `bin/magento setup:install`. The plugin needs to reinstall itself in the `<Magento>/var` directory, where it is required for Web Setup Wizard operations. |
docs/process_flows.md
Outdated
| 4. Magento calls [InstallData::install()](../src/Magento/ComposerRootUpdatePlugin/Setup/InstallData.php), [RecurringData::install()](../src/Magento/ComposerRootUpdatePlugin/Setup/RecurringData.php), or [UpgradeData::upgrade()](../src/Magento/ComposerRootUpdatePlugin/Setup/UpgradeData.php) as appropriate (which one depends on the specific `bin/magento setup` command and installed Magento version), which then calls [WebSetupWizardPluginInstaller::doVarInstall()](../src/Magento/ComposerRootUpdatePlugin/Setup/WebSetupWizardPluginInstaller.php) | ||
| 5. `WebSetupWizardPluginInstaller::doVarInstall()` finds the `magento/composer-root-update-plugin` version in the `composer.lock` file in the root Magento directory and calls `WebSetupWizardPluginInstaller::updateSetupWizardPlugin()` | ||
| 6. `WebSetupWizardPluginInstaller::updateSetupWizardPlugin()` checks the `<Magento>/var/vendor` directory for the `magento/composer-root-update-plugin` version installed there (if any) to see if it matches the version in the root `composer.lock` file | ||
| 7. If the version doesn't match or `magento/composer-root-update-plugin` is absent in `<Magento>/var/vendor`, `WebSetupWizardPluginInstaller::updateSetupWizardPlugin()` installs the root project's `magento/composer-root-update-plugin` version in a temporary directory then replaces `<Magento>/var/vendor` with the `vendor` directory from the temporary installation |
There was a problem hiding this comment.
Suggested change
| 7. If the version doesn't match or `magento/composer-root-update-plugin` is absent in `<Magento>/var/vendor`, `WebSetupWizardPluginInstaller::updateSetupWizardPlugin()` installs the root project's `magento/composer-root-update-plugin` version in a temporary directory then replaces `<Magento>/var/vendor` with the `vendor` directory from the temporary installation | |
| 7. If the version does not match or `magento/composer-root-update-plugin` is absent in `<Magento>/var/vendor`, `WebSetupWizardPluginInstaller::updateSetupWizardPlugin()` installs the root project's `magento/composer-root-update-plugin` version in a temporary directory then replaces `<Magento>/var/vendor` with the `vendor` directory from the temporary installation |
docs/process_flows.md
Outdated
|
|
||
| ## Explicit `var` installation command | ||
|
|
||
| **Scenario:** The user has cleared the `<Magento>/var` directory and wants to use the Web Setup Wizard to upgrade their Magento installation. The plugin needs to exist in `<Magento>/var` in order to function during Web Setup Wizard operations, so the user calls `composer magento-update-plugin install` to restore the plugin installation in the `<Magento>/var` directory. |
There was a problem hiding this comment.
Suggested change
| **Scenario:** The user has cleared the `<Magento>/var` directory and wants to use the Web Setup Wizard to upgrade their Magento installation. The plugin needs to exist in `<Magento>/var` in order to function during Web Setup Wizard operations, so the user calls `composer magento-update-plugin install` to restore the plugin installation in the `<Magento>/var` directory. | |
| **Scenario:** The user clears the `<Magento>/var` directory and wants to use the Web Setup Wizard to upgrade their Magento installation. The plugin must exist in the `<Magento>/var` directory, where it is required for Web Setup Wizard operations, so the user calls `composer magento-update-plugin install` to restore the plugin installation in the `<Magento>/var` directory. |
melnikovi
reviewed
Apr 12, 2019
README.md
Outdated
|
|
||
| ## Bypassing the plugin | ||
| To run the native `composer require` command without the plugin's updates, use the `--skip-magento-root` option. | ||
| To run the native `composer require` command without the plugin's updates, use the `--skip-magento-root-plugin` option. |
There was a problem hiding this comment.
What would be the use case for using plugin with setup wizard (question regarding statement below)? My understand is that people who use setup wizard don't have access to command line.
There was a problem hiding this comment.
I will go over this in a demo, but the plugin needs to be present for the setup wizard's dependency validation check or the check can fail for the same reasons why the plugin is necessary in the first place (root-level values can conflict with new Magento versions and Composer doesn't natively allow us to change them automatically).
In my opinion, the plugin is actually even more necessary for the setup wizard than a CLI-based upgrade specifically because people who use the wizard don't want to use the command line and without the plugin they're forced to. Our current upgrade process already requires CLI/filesystem access for upgrades to 2.3.x regardless of whether they're using the setup wizard or the command line (require-dev conflicts between 2.1/2.2 and 2.3 requirements).
The plugin allows us to reduce those existing upgrade steps to just a one-line install of the plugin. After that, those users' installations should never need command line operations for any similar conflicts we have in future releases.
melnikovi
reviewed
Apr 12, 2019
README.md
Outdated
| In this case, run the following command with the appropriate values to correct the existing `composer.json` file before proceeding with the expected `composer require` command for the target Magento product. | ||
|
|
||
| composer require <current_Magento_package> <current_version> --previous-magento-package <previous_Magento_package>=<previous_Magento_version> | ||
| composer require <current_Magento_package> <current_version> --base-magento-edition <community|enterprise> --base-magento-version <original_Magento_version> |
There was a problem hiding this comment.
I didn't finish code review of the plugin and don't have good understanding what it does. It's hard for me to understand the purpose from this overview and when I should use this command and the one below. I would suggest to add more description and maybe examples.
There was a problem hiding this comment.
Examples are a good thought, I will add those.
paliarush
suggested changes
Apr 29, 2019
There was a problem hiding this comment.
Done review of the Code portion. @melnikovi is reviewing the docs.
src/Magento/ComposerRootUpdatePlugin/Plugin/Commands/MageRootRequireCommand.php
Show resolved
Hide resolved
melnikovi
reviewed
Apr 30, 2019
|
|
||
| composer magento-update-plugin install | ||
|
|
||
| ## Example use case: Upgrading from Magento 2.2.8 to Magento 2.3.1 |
There was a problem hiding this comment.
Seems like this also exists in root readme? Maybe root readme should contain summary and a link to readme file in the module with more info?
There was a problem hiding this comment.
@jeff-matthews What is your opinion here?
melnikovi
approved these changes
Apr 30, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding
CONTRIBUTING.mdand developer documentation of classes and process flows and diagrams.Additionally, refactored ConflictResolver to DeltaResolver and split
--previous-magento-packageinto two options:--base-magento-editionand--base-magento-package.