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

Conversation

@henriquemattos
Copy link

Provide a narrative description of what you are trying to accomplish:

I think that this commit was not right implemented as the $pkck variable is used before it's declared.

  • [:question:] Are you fixing a bug?

    • Detail how the bug is invoked currently.
    • Detail the original, incorrect behavior.
    • Detail the new, expected behavior.
    • Base your feature on the master branch, and submit against that branch.
    • Add a regression test that demonstrates the bug, and proves the fix.
    • Add a CHANGELOG.md entry for the fix.
  • Are you creating a new feature?

    • Why is the new feature needed? What purpose does it serve?
    • How will users use the new feature?
    • Base your feature on the develop branch, and submit against that branch.
    • Add only one feature per pull request; split multiple features over multiple pull requests
    • Add tests for the new feature.
    • Add documentation for the new feature.
    • Add a CHANGELOG.md entry for the new feature.
  • Is this related to quality assurance?

  • Is this related to documentation?

@Xerkus Xerkus self-assigned this Oct 15, 2018
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@henriquemattos since the original code would crash without this change, it means that it wasn't ever tested: can you please add a test scenario that covers just this execution path, please?

@Xerkus
Copy link
Member

Xerkus commented Oct 18, 2018

@henriquemattos I took liberty to implement fix with tests in #341
Thank you for bringing it to our attention.
I will close this PR now.

@Xerkus Xerkus closed this Oct 18, 2018
@henriquemattos
Copy link
Author

Perfect, thank you everyone! ☺️

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