Skip to content
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

Only process versions as a version constraint #110

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fredden
Copy link

@fredden fredden commented Jul 11, 2023

When a patch is defined with a level and a source, the order of these is important but shouldn't be. When the source is defined first, then the patch is applied. However, when the level is defined first, then the patch is not applied. This seems to be related to the LabelVersionConfigComponent class interpreting the first item in the data array as a constraint. As the value is a number, this passes the "is this a version constraint" test, but because that version constraint doesn't match the installed version of the package, the patch is not applied.

Full composer.json showing problem
{
    "type": "project",
    "require": {
        "php": "~8.2.0",
        "magento/magento-coding-standard": "*",
        "vaimo/composer-patches": "^5.1"
    },
    "config": {
        "allow-plugins": {
            "dealerdirect/phpcodesniffer-composer-installer": true,
            "vaimo/composer-patches": true
        }
    },
    "extra": {
        "patches": {
            "magento/magento-coding-standard": {
                "$block->escape... versus $escaper->escape...": {
                    "level": "1",
                    "source": "https://github.com/magento/magento-coding-standard/pull/458.diff"
                }
            }
        }
    }
}

This is my first real pull request for this project. I've not been able to successfully run the test-suite locally. Please can I have some help creating a test for this problem. I'm not confident that this change is the correct way to fix this bug, nor that there are no knock-on effects. I'm hoping that the test-suite runs as expected here, and that I can adjust this pull request based on the results reported, however running the test-suite locally would be best.

@fredden fredden marked this pull request as ready for review July 11, 2023 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant