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

Override warning cleanup #570

Closed

Conversation

ianrrees
Copy link
Contributor

@ianrrees ianrrees commented Mar 1, 2017

This gets rid of ~5k warnings when building on MacOS.

@ianrrees ianrrees changed the title 20170301 override warning cleanup Override warning cleanup Mar 1, 2017
@wwmayer
Copy link
Contributor

wwmayer commented Mar 1, 2017

Hi Ian,

I have two questions:
How comes that for the macOS build on travis we don't get any of these warnings?
What is the reason to add a bunch of new macros instead of modifying the existing ones? For me it's not clear when to use which one when creating a new class.

@ianrrees
Copy link
Contributor Author

ianrrees commented Mar 1, 2017

Good questions! I should mention the other option is to just mute all the override warnings - I don't get the impression that this is a useful enough C++11 feature to justify too much work.

I think the Travis builds use an older clang. I've got XCode 8.2.1 (LLVM 8.0.0, clang-800.0.42.1) on my computer where I was seeing the warnings.

The macros are because the warnings only show when a class is declared that uses override at least once, for every method that is overridden but doesn't declare that it is an override. So, if the regular macros are changed to get overrides, then we need to put the override keyword on every overridden method in classes that use those macros. AFAIK, there's no problem with using the "wrong" macro, except that you'll get these warnings again.

@wwmayer
Copy link
Contributor

wwmayer commented Mar 1, 2017

OK I see. Thanks for the answer.

@wwmayer
Copy link
Contributor

wwmayer commented Mar 1, 2017

Merged.

@wwmayer wwmayer closed this Mar 1, 2017
@ianrrees
Copy link
Contributor Author

ianrrees commented Mar 1, 2017

Thanks! If you think it's worthwhile, I could look at moving the rest of the project to using the override keyword.

@ianrrees ianrrees deleted the 20170301-override-warning-cleanup branch March 2, 2017 04:33
@wwmayer
Copy link
Contributor

wwmayer commented Mar 2, 2017

Yes, if you still get warnings it would be good to properly fix them.

mosfet80 added a commit to mosfet80/FreeCAD that referenced this pull request Dec 14, 2023
4.5.0 - 2023-10-07
Features
requirements-txt-fixer: also sort constraints.txt by default.
FreeCAD#857 PR by @lev-blit.
FreeCAD#830 issue by @PLPeeters.
debug-statements: add bpdb debugger.
FreeCAD#942 PR by @mwip.
FreeCAD#941 issue by @mwip.
Fixes
file-contents-sorter: fix sorting an empty file.
FreeCAD#944 PR by @RoelAdriaans.
FreeCAD#935 issue by @paduszyk.
double-quote-string-fixer: don't rewrite inside f-strings in 3.12+.
FreeCAD#973 PR by @asottile.
FreeCAD#971 issue by @XuehaiPan.
Migrating
now requires python >= 3.8.
FreeCAD#926 PR by @asottile.
FreeCAD#927 PR by @asottile.
4.4.0 - 2022-11-23
Features
forbid-submodules: new hook which outright bans submodules.
FreeCAD#815 PR by @asottile.
FreeCAD#707 issue by @ChiefGokhlayeh.
4.3.0 - 2022-06-07
Features
check-executables-have-shebangs: use git config core.fileMode to determine if it should query git.
FreeCAD#730 PR by @Kurt-von-Laven.
name-tests-test: add --pytest-test-first test convention.
FreeCAD#779 PR by @asottile.
Fixes
check-shebang-scripts-are-executable: update windows instructions.
FreeCAD#774 PR by @mdeweerd.
FreeCAD#770 issue by @mdeweerd.
check-toml: use stdlib tomllib when available.
FreeCAD#771 PR by @DanielNoord.
FreeCAD#755 issue by @sognetic.
check-added-large-files: don't run on non-file stages.
FreeCAD#778 PR by @asottile.
FreeCAD#777 issue by @skyj.
4.2.0 - 2022-04-06
Features
name-tests-test: updated display text.
FreeCAD#713 PR by @asottile.
check-docstring-first: make output more parsable.
FreeCAD#748 PR by @asottile.
check-merge-conflict: make output more parsable.
FreeCAD#748 PR by @asottile.
debug-statements: make output more parsable.
FreeCAD#748 PR by @asottile.
Fixes
check-merge-conflict: fix detection of ====== conflict marker on windows.
FreeCAD#748 PR by @asottile.
Updating
Drop python<3.7.
FreeCAD#719 PR by @asottile.
Changed default branch from master to main.
FreeCAD#744 PR by @asottile.
4.1.0 - 2021-12-22
Features
debug-statements: add pdbr debugger.
FreeCAD#614 PR by @cansarigol.
detect-private-key: add detection for additional key types.
FreeCAD#658 PR by @ljmf00.
check-executables-have-shebangs: improve messaging on windows.
FreeCAD#689 PR by @pujitm.
FreeCAD#686 issue by @jmerdich.
check-added-large-files: support --enforce-all with git-lfs.
FreeCAD#674 PR by @amartani.
FreeCAD#560 issue by @jeremy-coulon.
Fixes
check-case-conflict: improve performance.
FreeCAD#626 PR by @guykisel.
FreeCAD#625 issue by @guykisel.
forbid-new-submodules: fix false-negatives for pre-push.
FreeCAD#619 PR by @m-khvoinitsky.
FreeCAD#609 issue by @m-khvoinitsky.
check-merge-conflict: fix execution in git worktrees.
FreeCAD#662 PR by @errsyn.
FreeCAD#638 issue by @daschuer.
Misc.
Normalize case of hook names and descriptions.
FreeCAD#671 PR by @dennisroche.
FreeCAD#673 PR by @revolter.
4.0.1 - 2021-05-16
Fixes
check-shebang-scripts-are-executable fix entry point.
FreeCAD#602 issue by @Person-93.
FreeCAD#603 PR by @scop.
4.0.0 - 2021-05-14
Features
check-json: report duplicate keys.
FreeCAD#558 PR by @AdityaKhursale.
FreeCAD#554 issue by @adamchainz.
no-commit-to-branch: add main to default blocked branches.
FreeCAD#565 PR by @ndevenish.
check-case-conflict: check conflicts in directory names as well.
FreeCAD#575 PR by @slsyy.
FreeCAD#70 issue by @andyjack.
check-vcs-permalinks: forbid other branch names.
FreeCAD#582 PR by @jack1142.
FreeCAD#581 issue by @jack1142.
check-shebang-scripts-are-executable: new hook which ensures shebang'd scripts are executable.
FreeCAD#545 PR by @scop.
Fixes
check-executables-have-shebangs: Short circuit shebang lookup on windows.
FreeCAD#544 PR by @scop.
requirements-txt-fixer: Fix comments which have indentation
FreeCAD#549 PR by @greshilov.
FreeCAD#548 issue by @greshilov.
pretty-format-json: write to stdout using UTF-8 encoding.
FreeCAD#571 PR by @jack1142.
FreeCAD#570 issue by @jack1142.
Use more inclusive language.
FreeCAD#599 PR by @asottile.
Breaking changes
Remove deprecated hooks: flake8, pyflakes, autopep8-wrapper.
FreeCAD#597 PR by @asottile.
chennes pushed a commit that referenced this pull request Dec 18, 2023
4.5.0 - 2023-10-07
Features
requirements-txt-fixer: also sort constraints.txt by default.
#857 PR by @lev-blit.
#830 issue by @PLPeeters.
debug-statements: add bpdb debugger.
#942 PR by @mwip.
#941 issue by @mwip.
Fixes
file-contents-sorter: fix sorting an empty file.
#944 PR by @RoelAdriaans.
#935 issue by @paduszyk.
double-quote-string-fixer: don't rewrite inside f-strings in 3.12+.
#973 PR by @asottile.
#971 issue by @XuehaiPan.
Migrating
now requires python >= 3.8.
#926 PR by @asottile.
#927 PR by @asottile.
4.4.0 - 2022-11-23
Features
forbid-submodules: new hook which outright bans submodules.
#815 PR by @asottile.
#707 issue by @ChiefGokhlayeh.
4.3.0 - 2022-06-07
Features
check-executables-have-shebangs: use git config core.fileMode to determine if it should query git.
#730 PR by @Kurt-von-Laven.
name-tests-test: add --pytest-test-first test convention.
#779 PR by @asottile.
Fixes
check-shebang-scripts-are-executable: update windows instructions.
#774 PR by @mdeweerd.
#770 issue by @mdeweerd.
check-toml: use stdlib tomllib when available.
#771 PR by @DanielNoord.
#755 issue by @sognetic.
check-added-large-files: don't run on non-file stages.
#778 PR by @asottile.
#777 issue by @skyj.
4.2.0 - 2022-04-06
Features
name-tests-test: updated display text.
#713 PR by @asottile.
check-docstring-first: make output more parsable.
#748 PR by @asottile.
check-merge-conflict: make output more parsable.
#748 PR by @asottile.
debug-statements: make output more parsable.
#748 PR by @asottile.
Fixes
check-merge-conflict: fix detection of ====== conflict marker on windows.
#748 PR by @asottile.
Updating
Drop python<3.7.
#719 PR by @asottile.
Changed default branch from master to main.
#744 PR by @asottile.
4.1.0 - 2021-12-22
Features
debug-statements: add pdbr debugger.
#614 PR by @cansarigol.
detect-private-key: add detection for additional key types.
#658 PR by @ljmf00.
check-executables-have-shebangs: improve messaging on windows.
#689 PR by @pujitm.
#686 issue by @jmerdich.
check-added-large-files: support --enforce-all with git-lfs.
#674 PR by @amartani.
#560 issue by @jeremy-coulon.
Fixes
check-case-conflict: improve performance.
#626 PR by @guykisel.
#625 issue by @guykisel.
forbid-new-submodules: fix false-negatives for pre-push.
#619 PR by @m-khvoinitsky.
#609 issue by @m-khvoinitsky.
check-merge-conflict: fix execution in git worktrees.
#662 PR by @errsyn.
#638 issue by @daschuer.
Misc.
Normalize case of hook names and descriptions.
#671 PR by @dennisroche.
#673 PR by @revolter.
4.0.1 - 2021-05-16
Fixes
check-shebang-scripts-are-executable fix entry point.
#602 issue by @Person-93.
#603 PR by @scop.
4.0.0 - 2021-05-14
Features
check-json: report duplicate keys.
#558 PR by @AdityaKhursale.
#554 issue by @adamchainz.
no-commit-to-branch: add main to default blocked branches.
#565 PR by @ndevenish.
check-case-conflict: check conflicts in directory names as well.
#575 PR by @slsyy.
#70 issue by @andyjack.
check-vcs-permalinks: forbid other branch names.
#582 PR by @jack1142.
#581 issue by @jack1142.
check-shebang-scripts-are-executable: new hook which ensures shebang'd scripts are executable.
#545 PR by @scop.
Fixes
check-executables-have-shebangs: Short circuit shebang lookup on windows.
#544 PR by @scop.
requirements-txt-fixer: Fix comments which have indentation
#549 PR by @greshilov.
#548 issue by @greshilov.
pretty-format-json: write to stdout using UTF-8 encoding.
#571 PR by @jack1142.
#570 issue by @jack1142.
Use more inclusive language.
#599 PR by @asottile.
Breaking changes
Remove deprecated hooks: flake8, pyflakes, autopep8-wrapper.
#597 PR by @asottile.
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