Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Minor comment update #285

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Minor comment update #285

wants to merge 4 commits into from

Conversation

bitwombat
Copy link

The statement in injectEventManager's comments that a shared event manager is injected into an existing event manager is incorrect - the code replaces the existing event manager with a new one. That's what I updated.

I may have added too much 'how it works' detail, but I puzzled over this a bit so I thought the explanation might help future readers.

@bitwombat
Copy link
Author

Also, I wonder about the pre-existing comment that "the conditional will always pass". I think that's true. Then does it need to be a conditional?

@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-mvc; a new issue has been opened at laminas/laminas-mvc#12.

@weierophinney
Copy link
Member

This repository has been moved to laminas/laminas-mvc. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

  • Squash all commits in your branch (git rebase -i origin/{branch})
  • Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
  • Run the laminas/laminas-migration tool on the code.
  • Clone laminas/laminas-mvc to another directory.
  • Copy the files from the second bullet point to the clone of laminas/laminas-mvc.
  • In your clone of laminas/laminas-mvc, commit the files, push to your fork, and open the new PR.
    We will be providing tooling via laminas/laminas-migration soon to help automate the process.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants