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

Remove unnecessary includes from clap.h #388

Closed

Conversation

Trinitou
Copy link
Contributor

let's see if this passes the pull-request workflows... 😉

I think this removes some clutter. here is my theory:

  • plugin.h is included via factory/plugin-factory.h and plugin extensions (makes sense to me)
    • host.h and plugin-features.h are included via plugin.h (makes sense as well)
  • universal-plugin-id.h is included via factory/preset-discovery.h (where it is actually used)

@Trinitou Trinitou mentioned this pull request Jan 18, 2024
@abique
Copy link
Contributor

abique commented Jan 18, 2024

I'm not sure about that one.

The goal of clap.h is to pull everything except the draft.
If x.h is included by y.h and we decide to not add x.h to clap.h, you still get x.h indirectly by including clap.h.
But, if one day x.h isn't included anymore from y.h, then we need to remember to add back x.h to clap.h.

If we directly include all headers in clap.h, we don't rely upon indirect inclusion.

We use #pragma once so I believe that it is a piece of cake for the compiler and the overhead is negligible.

Having everything in clap.h facilitate navigation within an IDE maybe 🤔 .

@baconpaul
Copy link
Collaborator

If clap.h is meant to be universal header I agree it should include things directly even if they are also included transitively

@Trinitou
Copy link
Contributor Author

Trinitou commented Jan 18, 2024

Should we close this (incomplete) pull-request as we want a more universal solution for all includes? Or should we leave it here and bring it into a 1.2.1 discussion later? + Maybe change it into a draft?

@baconpaul
Copy link
Collaborator

Let’s change it to draft so we don’t forget

@abique
Copy link
Contributor

abique commented Nov 1, 2024

So what do we decide here?
I was not in favor of this change, but I don't mind either.

What's the last call? We close it?

@Trinitou
Copy link
Contributor Author

Trinitou commented Nov 1, 2024

So what do we decide here? I was not in favor of this change, but I don't mind either.

What's the last call? We close it?

I'm fine with closing

@abique abique closed this Nov 1, 2024
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