Skip to content

Conversation

@matthewtrask
Copy link

No description provided.

@matthewtrask
Copy link
Author

@colinodell can you review this?

@colinodell
Copy link

colinodell commented Jan 2, 2019 via email

Copy link

@colinodell colinodell left a comment

Choose a reason for hiding this comment

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

Overall this looks pretty good - nice work! I did leave a few minor comments.

Also, at some point, I think we should maybe add tests for older PHP versions too to check for possible BC breaks.

@matthewtrask
Copy link
Author

@colinodell sorry it took a while, but these are taken care of.

Agreed on regression tests as well. Ill make an issue.

Copy link

@colinodell colinodell left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@matthewtrask matthewtrask merged commit b50590f into master Jan 10, 2019
@matthewtrask matthewtrask deleted the feature/modernize branch January 10, 2019 13:13
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.

3 participants