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

remove unnecessary <T> from valarray. #345

Closed
wants to merge 4 commits into from

Conversation

faithandbrave
Copy link
Contributor

add <T> of return type for consistency.

@faithandbrave faithandbrave changed the title fix missing <T> to valarray::operator=(initializer_list<T>) fix missing <T> to valarray::operator=(initializer_list<T>) Jul 23, 2014
@jwakely
Copy link
Member

jwakely commented Jul 23, 2014

My preference is to remove redundant template argument lists, it makes the code much easier to read, see e.g. ced4bbd

@faithandbrave
Copy link
Contributor Author

OK, I'll fix this pull request.

@jwakely
Copy link
Member

jwakely commented Jul 23, 2014

It might be worth waiting to see what @zygoloid and others think

@zygoloid
Copy link
Member

My preference is to leave out the template argument lists whenever naming the injected-class-name. Listing them explicitly makes the reader wonder whether they're somehow different.

@faithandbrave
Copy link
Contributor Author

I confirmed consensus. I'll remove <T>.

@faithandbrave
Copy link
Contributor Author

I re-pull requested. Please check the change.

@faithandbrave faithandbrave changed the title fix missing <T> to valarray::operator=(initializer_list<T>) remove unnecessary <T> from valarray. Jul 31, 2014
@@ -6885,7 +6885,7 @@
\indexlibrary{\idxcode{operator=}!\idxcode{valarray}}%
\indexlibrary{\idxcode{valarray}!\idxcode{operator=}}%
\begin{itemdecl}
valarray& operator=(initializer_list<T> il);
valarray<T>& operator=(initializer_list<T> il);
Copy link
Contributor

Choose a reason for hiding this comment

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

This <T> should be removed (i.e., this line shouldn't be part of the diff).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review. I removed the <T>.

@jwakely jwakely closed this in e1fffe9 Jan 7, 2015
@jwakely
Copy link
Member

jwakely commented Jan 7, 2015

Thanks - I committed a squashed merge of your changes.

@faithandbrave
Copy link
Contributor Author

Thanks!

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