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

Added the Whitespace style where the separator is a single whitespace #66

Merged
merged 3 commits into from
Nov 23, 2018

Conversation

wargamer
Copy link

As requested by issue #61

@tdwright
Copy link
Owner

HI @wargamer!

Thanks for this PR. From a superficial look, everything looks fine. I see you've added a new conformance test to go with the new style, which is great. I'll go through it all properly this weekend when I'm in front of my laptop.

The only thing that I'm not sure about is the name. I was going to raise this on the issue (#61) itself, but you were too quick with this PR! 😉 "Empty" is what the reporting user initially suggested, but I'm not sure it's quite right. To me it feels like it almost implies there will be no data in the table. What do you think? Some ideas I had were:

  • Whitespace
  • Blanks
  • Gaps
  • Ghost

I'm also open to any suggestions you (or others?) might have. Or, if you really like "Empty", we can stick with that.

Tom

@wargamer
Copy link
Author

Hi Tom,

You make a fair point, hadn't thought of it that way.
I went with Whitespace since that seems most descriptive/fitting out of the four.

@tdwright
Copy link
Owner

Ah brilliant. I personally think that name works loads better. And thanks for the quick turnaround.

I'll get this properly reviewed ASAP, but (like I said) it'll probably be over the weekend whilst I'm working on #60

Cheers,
Tom

@wargamer wargamer changed the title Added the Empty style where the separator is a single whitespace Added the Whitespace style where the separator is a single whitespace Nov 23, 2018
Copy link
Owner

@tdwright tdwright left a comment

Choose a reason for hiding this comment

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

All good! 💯

Thanks for your contribution @wargamer!

@tdwright tdwright merged commit 2ea451e into tdwright:develop Nov 23, 2018
@wargamer wargamer deleted the feature/empty-style branch November 24, 2018 18:48
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