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

logger: do not trim prefix #231

Merged
merged 1 commit into from
Jun 16, 2020
Merged

logger: do not trim prefix #231

merged 1 commit into from
Jun 16, 2020

Conversation

McAllaster
Copy link
Contributor

The logger should preserve any leading/trailing white space provided in the prefix

Closes #227

Side note: I'm keeping boilerplate code here just because I see it within the rest of the logger specification. Normally I'd DRY it up a bit with test generators or a beforeEach, but I'm unsure if that is a welcome change.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 445cced on McAllaster:prefix-spacing into 54b6456 on kimmobrunfeldt:master.

Copy link
Member

@gustavohenke gustavohenke left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!
I actually checked old code to see if prefix trimming ever was a thing, and no. I introduced it out of the blue with the programmatic API 🤔


Normally I'd DRY it up a bit with test generators or a beforeEach, but I'm unsure if that is a welcome change.

It would yes! As long as it's done in a separate commit/PR, it's all good 👍

@gustavohenke gustavohenke merged commit e837544 into open-cli-tools:master Jun 16, 2020
@gustavohenke
Copy link
Member

Proceeded with the merge in case you don't have time/don't feel like tidying up the tests.

@McAllaster McAllaster deleted the prefix-spacing branch June 17, 2020 23:51
@McAllaster McAllaster mentioned this pull request Jun 17, 2020
@gustavohenke
Copy link
Member

Release as v5.3.0!

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.

Allow spaces at prefix start/end
3 participants