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

add move constructor and move assignment for ann modules #491

Merged
merged 2 commits into from Dec 31, 2015

Conversation

stereomatchingkiss
Copy link
Contributor

Implement move constructor and move assignment for ann modues.

By now only implement for linear_layer.hpp, base_layer.hpp and bias_layer.hpp, will implement for another classes step by step

@zoq
Copy link
Member

zoq commented Dec 10, 2015

Wow that was fast, thanks for the first modifications. Should I merge the modification as it is or should I wait for more changes? It's up to you, pick the one that is easier for you. If it's easier for you to open a new pull request, I go and merge the pull request.

@stereomatchingkiss
Copy link
Contributor Author

Should I merge the modification as it is or should I wait for more changes

You could merge it now if you think it is ok.

} // namespace ann
} // namespace mlpack
}; // namespace ann
}; // namespace mlpack
Copy link
Member

Choose a reason for hiding this comment

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

This is pedantic (literally), but if you compile with -pedantic the extra semicolon produces a warning. So I've been trying to avoid doing that. :)

zoq added a commit that referenced this pull request Dec 31, 2015
Add move constructor and move assignment for ann modules.
@zoq zoq merged commit 29c0405 into mlpack:master Dec 31, 2015
@zoq
Copy link
Member

zoq commented Dec 31, 2015

Thanks again for the contribution! I removed the semicolons which produces a warning when compiling with the -pedantic option as @rcurtin pointed out in 6945c11. Let me know if I messed anything up.

@stereomatchingkiss stereomatchingkiss deleted the ann_move branch January 4, 2016 05:00
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

3 participants