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

(trivial) Remove using namespace std and add std:: qualifications in few places where needed #2092

Merged
merged 1 commit into from Jul 11, 2022

Conversation

p12tic
Copy link
Contributor

@p12tic p12tic commented Jul 10, 2022

using namespace std; is generally an antipattern as it imports a huge number of common names into the global namespace. We already qualify names with std:: in majority of cases and using namespace std is not actually needed most of the time. This commit addresses a small number of remaining cases where std:: is missing and removes all using namespace std.

Copy link
Member

@pmoulon pmoulon left a comment

Choose a reason for hiding this comment

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

Thank you for your commit.
I made just a minor comment to exclude from this PR the modification of the sensor database.
I also launched the continuous integration and we will see what various compiler says ;-)

@@ -15,6 +15,7 @@ Acer CS-5531;5.75
Acer CS-6530;5.75
Acer CS-6531;5.75
Acer CU-6530;5.75
samsung SM-G973F;5.75
Copy link
Member

Choose a reason for hiding this comment

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

Could we remove this modification from this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, this was included due to an oversight. Fixed now.

`using namespace std;` is generally an antipattern as it imports a huge
number of common names into the global namespace. We already qualify
names with std:: in majority of cases and `using namespace std` is not
actually needed most of the time. This commit addresses a small number
of remaining cases where std:: is missing and removes all `using
namespace std`.
@p12tic p12tic force-pushed the remove-using-namespace-std branch from b0e20bf to 767b8c0 Compare July 11, 2022 13:32
@pmoulon
Copy link
Member

pmoulon commented Jul 11, 2022

Thank you. Everything looks good. Thank you for the PR, the contribution. Looking forward your next contribution 😀

@pmoulon pmoulon merged commit 08ac774 into openMVG:develop Jul 11, 2022
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

2 participants