wikimedia / composer-merge-plugin

Merge one or more additional composer.json files at Composer runtime
MIT License
934 stars 159 forks source link

Pre-Merge and post-Merge commands #151

Closed phayes closed 3 years ago

phayes commented 7 years ago

This pull request adds both pre-merge and post-merge command hooks.

Specifically, it adds: pre-merge-require-cmd pre-merge-optional-cmd post-merge-required-cmd post-merge-optional-cmd

This allows users to trigger scripts just before a merge happens or just after a merge happens. For example:

    "scripts": {
        "pre-merge-require-cmd": [
            "\\MyProject\\composer\\ScriptHandler::myPreMergeCommand"
        ]
    }
phayes commented 7 years ago

I'm a little lost on the error that Travis is reporting. Any guidance on how to update the tests to account for this change?

bd808 commented 7 years ago

I'm a little lost on the error that Travis is reporting. Any guidance on how to update the tests to account for this change?

The $this->composer member used in the tests is a Prophecy mock and not a live object. You need to update all the tests that will trigger calls to $this->composer->getEventDispatcher()->dispatchScript(...) by the plugin to return a mock when $this->composer->getEventDispatcher() is called and then have that mock expect the appropriate calls from the plugin code.


This patch also needs documentation updates if we land it. Features that are only discoverable by reading the source code are not generally useful.


I'm not fundamentally against this relatively small addition, but it is nice to have an associated issue that documents the intended use-case and allows for discussion. Inevitably someone will file a bug saying that this new feature doesn't work for their use-case of X and it is nice to be able to look and see if this is something that we reasonably hoped would be possible when introducing the feature or not.

bd808 commented 3 years ago

Closed for inactivity.