-
Notifications
You must be signed in to change notification settings - Fork 118
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
fix(server): improved packfile encoding to support short messages and… #409
Conversation
… unicode in errors
✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.
|
Thanks for the PR @sammoorhouse ! Please reach out to [email protected] if you encounter any issue to sign the FINOS CLA using EasyCLA. |
@sammoorhouse - thanks for the contribution 👍 I'd been wondering how to get some "prettier" messages into the CLI for the developer. Do you think we could include a test case with this PR to demonstrate it works successfully? Perhaps to demonstrate that a message successfully returns with Unicode characters? Happy to help if needed 🎉 |
Thanks @JamieSlome I'll have a look later today! |
I extracted 'handleMessage' in proxy/routes and wrote two tests just for that function in a second commit just now. Thanks! |
Can someone kick the build again? It failed on something unrelated imho. |
The failure is due to #213, we have some sort of repo permission bug in the coverage steps. I’ll try to get a PR open soon to get it fixed then we can rebase on this one. Seems like a simple enough fix: zgosalvez/github-actions-report-lcov#150 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🍰
Ready to go once we've addressed the code coverage permissions issue...
fix(server): improved packfile encoding to support short messages and…
… unicode in errors
This change allows you to say
in a step.
Also fixes a padding bug that meant fewer than 5chars of errors broke the proxy response.