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

Duplicate include removed #10099

Merged
merged 2 commits into from Jul 10, 2016

Conversation

@drieschel
Copy link
Contributor

commented Jul 10, 2016

The AMLUtils.h was included twice. Don't see a benefit for.

Also <string.h> and are included. Could one of the two statements get removed?

@Paxxi

This comment has been minimized.

Copy link
Member

commented Jul 10, 2016

sorry to say this but you removed the wrong one :) That one should stay and the utils/AMLUtils.h should go. <string> and <string.h> are different files, the first is the c++ include for std::string and the second are for C string functions.

@Paxxi Paxxi added the Type: Cleanup label Jul 10, 2016
@drieschel

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2016

I think it doesn't matter which one have to get removed, because both versions link to the same folder. So should get the other "#include utils/*.h" statements changed to "#include *.h" ?

Edit: I see the difference ^^

@drieschel

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2016

@Paxxi Now it should be okay.

PS: I don't mind making mistakes. It's the only way to learn. :D

@Paxxi

This comment has been minimized.

Copy link
Member

commented Jul 10, 2016

Thanks :)

There's a convention when it comes to #include and generally in a .cpp file the first include should be the corresponding header file without a path. Having it first catches many errors and not having a path means you can freely move the files and they'll find each other correctly

@Paxxi Paxxi merged commit a6a8c5b into xbmc:master Jul 10, 2016
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@Paxxi Paxxi added this to the Krypton 17.0-alpha3 milestone Jul 10, 2016
@drieschel drieschel deleted the drieschel:amlutils_duplicate_include branch Jul 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.