Skip to content

consistent table headlines #4435

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

Merged
merged 2 commits into from
Nov 13, 2014
Merged

consistent table headlines #4435

merged 2 commits into from
Nov 13, 2014

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Nov 7, 2014

Q A
Doc fix? yes
New docs? no
Applies to all
Fixed tickets

@wouterj
Copy link
Member

wouterj commented Nov 7, 2014

👍 Cool. you're always blazing fast on these things!

@xabbuh
Copy link
Member Author

xabbuh commented Nov 7, 2014

Otherwise I would forget them. ;)

@javiereguiluz
Copy link
Member

@xabbuh thanks for working on this!

I have a comment about the new table syntax. In this table, columns are separated with two white spaces, while in the rest of the tables you use one white space.

Instead of suggesting you to use also one white space on the first table, I'd like to ask you reconsider using always two white spaces. That's what I've been using and I think that it improves readability.

@xabbuh
Copy link
Member Author

xabbuh commented Nov 11, 2014

@javiereguiluz There was no specific reason. Honestly, I simply didn't realise that there already were two spaces. Personally, I don't really care if we use one space or two spaces.

@wouterj @weaverryan What are your thoughts on this?

@wouterj
Copy link
Member

wouterj commented Nov 11, 2014

+1 for 2 spaces

Op di 11 nov. 2014 18:33 schreef Christian Flothmann <
notifications@github.com>:

@javiereguiluz https://github.com/javiereguiluz There was no specific
reason. Honestly, I simply didn't realise that there already were two
spaces. Personally, I don't really care if we use one space or two spaces.

@wouterj https://github.com/WouterJ @weaverryan
https://github.com/weaverryan What are your thoughts on this?


Reply to this email directly or view it on GitHub
#4435 (comment).

@weaverryan
Copy link
Member

If both render ok, I'm also cool with 2 spaces - I guess the extra separation makes it a bit more readable in the source.

@xabbuh
Copy link
Member Author

xabbuh commented Nov 13, 2014

Alright, do we wanna do that in this pull request or do you prefer a different one?

@weaverryan
Copy link
Member

@xabbuh This one if it's ok - it'll save me from merging up twice, which may involve a few conflicts :).

@xabbuh
Copy link
Member Author

xabbuh commented Nov 13, 2014

Of course, no problem. Here we go.

@weaverryan weaverryan merged commit 10b607b into symfony:2.3 Nov 13, 2014
weaverryan added a commit that referenced this pull request Nov 13, 2014
This PR was merged into the 2.3 branch.

Discussion
----------

consistent table headlines

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets |

Commits
-------

10b607b separate table columns with two spaces
178a2d6 consistent table headlines
@weaverryan
Copy link
Member

@xabbuh, beautiful - merged in and merged up to master!

@xabbuh xabbuh deleted the table-headlines branch November 13, 2014 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants