Closed mischah closed 8 years ago
I would be super happy if @sindresorhus could review that as soon as possible (He’s the one who helped me with #23)
Cause I would love to merge that into master and prepare a release as soon as possible to get these bugfixes out on the street.
You want me to squash the 3 commits before that PR lands into master?
No worries at all. The code here is very complex.
Could you add the above Node.js script as manual-test.js
to the repo? So we can run the manual tests to verify whenever.
You want me to squash the 3 commits before that PR lands into master?
No need. GitHub has a squash and merge button now.
I’ve just added the manual-test.js
file.
Let me know if there is anything else I can do.
Otherwise I would:
Awesome. Thanks for fixing :)
opps. you’ve done that while I was typing.
Published.
@mischah Could you do 4.
? I'm out of time.
You were too fast.
Build fails in older node versions because I used const
in manual-test.js
:dizzy_face:
Already fixed
ah. okay. man you’re fast like a 🚀
Add Release notes to the Release tab here on Github
Done. Thanks for everything. Highly appreciated what you are doing 💖
Yo @yeoman/yosay,
This PR fixes two (not so edgy) edge case issues I introcuded with #23.
First of all let me say how sorry and ashamed I am for creating these bugs :confounded:
Bug No. 1
The overflowing of messages that are too long is producing corrupted output of speech bubble when using the
maxLength
option.This is fixed with:
See screenshot:
Calling
yosay
more than once within a script adaptingtopOffset
causes corrupted output of the latter speech bubbles.This is fixed with:
See screenshot:
Conclusion
As said before I am terribly sorry to messed this up with the introduction of the speech bubble tip.
It was caused by not thoroughly hand testing and the auto generating of test fixtures :anguished:
I made sure to hand test the fixes of this PR by with this node file:
And this bash file: