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

Increase post install/update handler priority to support other plugins #153

Merged
merged 2 commits into from
Oct 2, 2017

Conversation

mmenozzi
Copy link
Contributor

@mmenozzi mmenozzi commented Sep 11, 2017

There are many other Composer's plugins that perform install operation on packages after they have been installed. Many times this operation involves coping packages files elsewhere (look at https://github.com/AydinHassan/magento-core-composer-installer for an example). In such situation it's useful to have patches applied BEFORE the install/copy operation. So, this PR increase the post install/update event handler's priority so other plugins (with priority=0) to run after that patches have been applied.

This makes other plugins (with priority=0) to run after that patches
have been applied.
@mmenozzi
Copy link
Contributor Author

Hi @cweagans,
do you plan to merge this?
If something is not clear let me know.
Thanks.

@cweagans
Copy link
Owner

cweagans commented Oct 2, 2017

I don't have an immediate objection to this, but at a minimum, this needs a comment about why the weight is increased so that it doesn't get blown away later.

@mmenozzi
Copy link
Contributor Author

mmenozzi commented Oct 2, 2017

HI @cweagans, you mean a comment in the code?

@cweagans
Copy link
Owner

cweagans commented Oct 2, 2017

Yes. Just right above the lines where the weight is increased. Something to the effect of "This is a higher weight for compatibility with [plugin]"

@cweagans cweagans closed this Oct 2, 2017
@mmenozzi
Copy link
Contributor Author

mmenozzi commented Oct 2, 2017

@cweagans Ok I'll do it but reopen this pull request please because I'll update the priority-support branch 😄

@cweagans
Copy link
Owner

cweagans commented Oct 2, 2017

Ah, sorry. Was closing some others. Closed this by mistake.

@cweagans cweagans reopened this Oct 2, 2017
@mmenozzi
Copy link
Contributor Author

mmenozzi commented Oct 2, 2017

@cweagans done!

@cweagans cweagans merged commit cc88ad5 into cweagans:master Oct 2, 2017
@cweagans
Copy link
Owner

cweagans commented Oct 2, 2017

Thanks!

@mmenozzi
Copy link
Contributor Author

mmenozzi commented Oct 2, 2017

Thank you to you for the merge!
Do you already know when you'll release a new "stable" version with this added feature?

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.

2 participants