Make it possible to exclude classes (directories) for compilation #986
Conversation
|
@Flyingmana, thank you for this contribution. We are going to validate it and make a decision about supporting such cases. |
|
please consider such a case is not avoidable if you want to have modules which are able to run in Magento1 and Magento2 |
| ] | ||
| ); | ||
| $opt->parse(); | ||
|
|
||
| $generationDir = $opt->getOption('generation') ? $opt->getOption('generation') : $rootDir . '/var/generation'; | ||
| $diDir = $opt->getOption('di') ? $opt->getOption('di') : $rootDir . '/var/di'; | ||
| $fileExcludePatterns = $opt->getOption('exclude-pattern') ? [$opt->getOption('exclude-pattern')] : ['/\/m1\//']; |
There was a problem hiding this comment.
@Flyingmana
Is "m1" as default exclude pattern Ok?
Does it make sense event specify default? Maybe better to keep it empty?
Thoughts?
There was a problem hiding this comment.
I think a default is important, so other module vendors can use this as standard and know their module works without the need of manual action from the users. "/m1/" for this is a simple one, and should not conflict with any other path
I thought about it, but I dont find another solution which would work automatic without bigger changes or a per module config in the compiler.
|
Builds: green |
|
@Flyingmana, one question about changes as a result of code review. Do you think we really need to process excluded pattern as array? When we run compiler.php we'll get it as string. |
|
@vpelipenko I dont think its important to be an array, but as I did not know how you maybe want to change or extend the behavior, I decided to make it as extensible as possible. But if it is an array or not doesn't matter for me |
|
Is there anything else I can help with now? |
|
This PR has some conflicts with our internal code, due to last improvements in code compilation tools, and can be accepted as is. Let's wait for the next public update where these changes will become live. Definitely you will need to do some changes in this PR after that. |
|
have rebased the PR, tested locally and Travis is still green. |
|
@Flyingmana, our internal improvements in code compilation tools are already available in public GitHub. As it was expected before, this PR has conflicts with code in develop branch: you can see, for example, that dev/tools/Magento/Tools/Di/Compiler/Directory.php was removed. But we still see valuable the possibility to exclude directories for compilation. Sorry for that, but could you update this PR again? Also it will good to support additionally pattern \m1\ (with back slashes) and make it case insensible. |
|
No problem :) You are right, back slashes are important because of windows environments, that makes the pattern a bit more ugly. I kept it as single pattern, but we could split it into two patterns in theory. Have it already working locally, will push it in a few minutes |
Always has the same result, so repeated execution was not necessary
|
CR: passed |
Make it possible to exclude classes (directories) for compilation
Story - MAGETWO-66799 Remove legacy implementation of static content deployment
…roduct visibility. #986
[EngCom] Public Pull Requests - 2.2-develop - MAGETWO-85311: Added namespace to product videos fotorama events #12469 #991 - MAGETWO-85300: 8437: Silent error when an email template is not found #970 - MAGETWO-85293: 12613: Verbiage Update Required: Product Image Watermark size Validation Message. #985 - MAGETWO-85286: 8176: LinkManagement::getChildren() does not include product visibility. #986 - MAGETWO-85285: 12482: Sitemap image links in MultiStore #935 - MAGETWO-84955: Set Current Store from Store Code if isUseStoreInUrl #12529 - MAGETWO-84764: NewRelic: Disables Module Deployments, Creates new Deploy Marker Command #12477 - MAGETWO-84439: 12180 Remove unnecessary use operator for Context, causes 503 error i… #12220
MSI-874: Api-Functional Tests For MSI are Failed.
Reason is:
A single module which is working for Magento 1.x and 2.x
the 2.x specific files are not problem for 1.x, but the 1.x specific files will get parsed from the compiler causing two errors.
To prevent this errors this patch allows to specify an ignore pattern with
/m1/as default, so everything a module places in a m1 directory, is not parsed from the compiler anymore.I will provide documentation after this get merged, not sure about tests yet, but will see later how to add some for it