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

clang-tidy stuff #1237

Merged
merged 2 commits into from
Mar 1, 2021
Merged

clang-tidy stuff #1237

merged 2 commits into from
Mar 1, 2021

Conversation

neheb
Copy link
Contributor

@neheb neheb commented Feb 18, 2021

last commit was done manually. I pay have messed something up.

Copy link
Member

@KarlStraussberger KarlStraussberger left a comment

Choose a reason for hiding this comment

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

Some of the removed refs are breaking.

src/config/config_setup.h Outdated Show resolved Hide resolved
src/config/config_setup.cc Outdated Show resolved Hide resolved
src/database/database.h Outdated Show resolved Hide resolved
src/database/sql_database.cc Outdated Show resolved Hide resolved
@KarlStraussberger
Copy link
Member

It's still breaking. Most of the refs in config_setup are return values.

@whyman
Copy link
Member

whyman commented Feb 23, 2021

Did the changes that @KarlStraussberger ask for make it in? Doesnt look like more commits have been added?

@neheb
Copy link
Contributor Author

neheb commented Feb 23, 2021

Updated.

src/content/content_manager.cc Outdated Show resolved Hide resolved
src/content/content_manager.cc Outdated Show resolved Hide resolved
src/database/database.cc Outdated Show resolved Hide resolved
src/metadata/matroska_handler.cc Outdated Show resolved Hide resolved
src/metadata/matroska_handler.cc Outdated Show resolved Hide resolved
void fillMetadata(std::shared_ptr<CdsObject> obj) override;
std::unique_ptr<IOHandler> serveContent(std::shared_ptr<CdsObject> obj, int resNum) override;
void fillMetadata(std::shared_ptr<CdsObject> item) override;
std::unique_ptr<IOHandler> serveContent(std::shared_ptr<CdsObject> item, int resNum) override;

Copy link
Member

Choose a reason for hiding this comment

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

It's CdsObject so the paramter should be named obj. Maybe it's not done in MetadataHnadler

Copy link
Member

Choose a reason for hiding this comment

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

Please rename the arguments in implentation. Declaration is as it should be.

@KarlStraussberger
Copy link
Member

Updated.

There are a lot more breaking changes. I'd prefer to leave those references as they were.

@neheb
Copy link
Contributor Author

neheb commented Feb 27, 2021

I removed the reference changes.

@KarlStraussberger
Copy link
Member

I also commented on the renamed arguments. The parameters in the headers are fine. The code should be renamed. item -> obj instead of vice versa.

Found with google-explicit-constructor

Signed-off-by: Rosen Penev <rosenp@gmail.com>
Found with google-build-using-namespace

Signed-off-by: Rosen Penev <rosenp@gmail.com>
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