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

Harmonize spacing for template headers. #1278

Merged
merged 1 commit into from Nov 27, 2017
Merged

Conversation

jensmaurer
Copy link
Member

@jensmaurer jensmaurer commented Dec 20, 2016

Use template<class T>.
Also remove space between two closing template brackets in [lib].
Leave [temp] unchanged, intentionally showing a variety of styles.

Fixes #53.

@jensmaurer
Copy link
Member Author

Rebased.

@jwakely
Copy link
Member

jwakely commented Mar 5, 2017

This looks good to me.

@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 5, 2017

All checks have failed! How many more checks could one fail?

@jensmaurer
Copy link
Member Author

There's only one check, and it's the check-whitespace check, apparently preventing trailing whitespace.

@jensmaurer
Copy link
Member Author

Let's try again.

@tkoeppe
Copy link
Contributor

tkoeppe commented Oct 3, 2017

I imagine this PR would need some rebasing, but I'm not sure if we want a monolithic commit here. @zygoloid, how would you like to approach this?

@@ -555,13 +555,13 @@
BidirectionalIterator first,
BidirectionalIterator last,
Predicate pred);
template <class InputIterator, class OutputIterator1,
template<class InputIterator, class OutputIterator1,
class OutputIterator2, class Predicate>
Copy link
Member

Choose a reason for hiding this comment

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

This line and those like it need to have their alignment fixed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I've scriptified the search and found a few pre-existing misalignments, too. Fixed.

@zygoloid
Copy link
Member

zygoloid commented Oct 9, 2017

The difference between one commit per file and one commit in total seems inconsequential to me from a review perspective. I'm fine with letting the change author make that call.

@zygoloid zygoloid added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Nov 12, 2017
@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 21, 2017

Rebase?

@jensmaurer
Copy link
Member Author

This needs more than a rebase. I also need to review all the alignments (see @zygoloid's comment).

Use 'template<class T>'.
Also remove space between two closing template brackets.
@jensmaurer jensmaurer removed the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Nov 24, 2017
@jensmaurer
Copy link
Member Author

Rebased (in fact, re-done) and misalignments fixed.

@zygoloid zygoloid merged commit e543af3 into cplusplus:master Nov 27, 2017
@jensmaurer jensmaurer deleted the b23 branch November 27, 2017 21:28
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.

None yet

4 participants