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

264 reserved header include guard #268

Closed

Conversation

guitorri
Copy link
Member

@guitorri guitorri commented Jun 1, 2015

Fix all header include guards. The leading underscore is reserved to the C++ implementation. Uppercase is a common convention. This closes issue #264.

@guitorri guitorri force-pushed the 264-reserved-header-include-guard branch 2 times, most recently from 89f50da to 8e47d6a Compare June 1, 2015 22:52
@guitorri
Copy link
Member Author

Some headers are quite generic (VARIABLE_H, ANALYSIS_H, APPLICATIONS_H). If we ever become serious about an API, this could be a problem.
Should we prefix all header include guards with something particular to Qucs (QUCS_)?

@in3otd
Copy link
Contributor

in3otd commented Jul 3, 2015

makes sense anyway... I saw around that some proposes to use macro names like PROJECT_DIR_HEADERNAME so we will have something like QUCS-CORE_COMPONENTS_MICROSTRIP_MSBEND_H ...this is maybe a bit too much.

@guitorri
Copy link
Member Author

guitorri commented Jul 3, 2015

Perhaps just PROJECT_HEADERNAME ?

@in3otd
Copy link
Contributor

in3otd commented Jul 3, 2015

yes, just PROJECT_HEADERNAME seems better for Qucs.

(Somewhat unrelated, maybe one day we should also change the qucs-core directory name to qucsator, since we are going to support several simulators)

@guitorri guitorri added this to the 0.0.20 milestone Jul 18, 2015
@in3otd in3otd mentioned this pull request Sep 29, 2015
@guitorri guitorri mentioned this pull request Feb 14, 2016
7 tasks
@guitorri guitorri changed the base branch from master to develop February 18, 2017 12:14
Header include guards started with undercore are reserved to the
implementation. Fixing for the sake of good practice

Spotted by Markus Elfring. See issue Qucs#264.
This is just a convention. Doesn't hurt to be consistent.
@guitorri guitorri force-pushed the 264-reserved-header-include-guard branch from 8e47d6a to cbb8aad Compare February 18, 2017 13:19
@guitorri
Copy link
Member Author

Rebased.
I will do to theQUCS_HEADERNAME throughout.
And revert to $(MODULE)_H to VA_$(module)_H` for adms/veriloga files

@felix-salfelder
Copy link
Member

hmm, any reason, we should not switch header guards to #pragma once? seems to be widely supported, and less messy.

@guitorri
Copy link
Member Author

Didn't know about it.
Got mixed feeling after reading this:
http://stackoverflow.com/questions/1143936/pragma-once-vs-include-guards

@felix-salfelder
Copy link
Member

yes, there may be pitfalls (that were not entirely clear to me).
if conventional guards works, we should maybe stick to it.

iirc, we had once fixed output filenames, to get $(FILE).cpp from $(FILE).va... using that logic, wouldn't VA_$(FILE)_H, or VA_$(FILE)_$(MODULE)_H make (a bit) more sense?

@guitorri guitorri modified the milestones: 0.0.20, 0.0.21 Oct 4, 2017
@felix-salfelder
Copy link
Member

Most of this is Qucsator (it needs rebase). The other bits seem obsolete, as most plugins don't need header guards.

@felix-salfelder felix-salfelder deleted the branch Qucs:develop July 2, 2024 08:22
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.

3 participants