Skip to content

Modernize the codebase #11

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Petross404
Copy link
Contributor

This PR applies the default keywords to empty dtors, Q_DISABLE_COPY_MOVE macro to specifically delete any move or copy operation, const qualifies some variables and functions etc.

@arturbac arturbac self-requested a review January 15, 2025 18:47
@arturbac arturbac self-assigned this Jan 15, 2025
Copy link
Owner

@arturbac arturbac left a comment

Choose a reason for hiding this comment

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

I am not allowed to apply clang-format, so do it Yourself pls or let me push into PR.

@Petross404
Copy link
Contributor Author

I am not allowed to apply clang-format, so do it Yourself pls or let me push into PR.

I am happy to help. Should I format the changed files only and run git commit --amend --no-edit?

@arturbac
Copy link
Owner

nvm Ill reformat code later.

@arturbac
Copy link
Owner

could You enable allow edit for me in PR, it is in Your options

@Petross404
Copy link
Contributor Author

image

Like this?

@arturbac
Copy link
Owner

arturbac commented Jan 16, 2025

Like this?

sure thx.
Btw I do not like "pedantic" const correctness on everything, I never make errors "use after move" but it very often causes prevention of copy elision. Sometimes for complex code I use const when I explicitly want to know variable was not modified but not in such approach as in this PR, this is redundant and makes code less readable and possibly prevents copy elision for named variables if applicable.
Anyway it does nothing valuable in this code, but since You did that just make sure there is no such case when const named variable is returned from function and previously it was not const.

@Petross404
Copy link
Contributor Author

Anyway it does nothing valuable in this code, but since You did that just make sure there is no such case when const named variable is returned from function and previously it was not const.

I suspect the compiler would rant about it? I might got carried away from clang-tidy's proposals and best practices. Whatever, I can remove them, it's not like it's the improvement or something.

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.

2 participants