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

feat: truncate to length + tests #5

Closed
wants to merge 1 commit into from
Closed

feat: truncate to length + tests #5

wants to merge 1 commit into from

Conversation

atymic
Copy link

@atymic atymic commented Jul 14, 2021

I'll add docs after your review :)

@chrisminett
Copy link
Member

Thanks @atymic ! I'll take a proper look in the next week. First thoughts (may be some wrong assumptions here from a quick glance!):

  • Glad to see it keeps immutability
  • I agree with the changes to "expectedException", but that's out-of-scope for this change.
    • Current code is for PHP 5.6+, but I've been going through all Phlib repos in the past few weeks to make them ^7.3|^8.0 at a minimum, which includes upgrading PHPUnit to v9 and making this exception change amongst others. I'll do this repo next, then this PR could be rebased from that.
  • I wonder about keeping the truncate behaviour in a separate class, so that the existing length class can just be about the inspection. It is useful that the truncate() method returns a new instance which will have all the same inspection methods on it though, so I'm not sure what the wiring would look like for that.

@chrisminett chrisminett self-assigned this Jul 14, 2021
@chrisminett chrisminett self-requested a review July 14, 2021 08:41
@chrisminett
Copy link
Member

Closes #4

@atymic
Copy link
Author

atymic commented Jul 19, 2021

See #9 same situ as other PR, sorry.

@atymic atymic closed this Jul 19, 2021
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