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

Add dependencies using EXCLUDE_FROM_ALL flag #198

Merged
merged 7 commits into from
Feb 16, 2021
Merged

Conversation

TheLartians
Copy link
Member

@TheLartians TheLartians commented Feb 15, 2021

Implements #152 by adding all dependencies using the EXCLUDE_FROM_ALL flag by default adding an EXCLUDE_FROM_ALL optional argument. Further testing is required to see this could represent a breaking change for real projects and see if an optional implementation or an opt-out flag would be appropriate.

TODO

  • Test in real-world projects
  • Decide if should be always on, opt-in or opt-out
  • Settle for an API
  • Update cmake-format configuration
  • Consider edge cases
  • Add a unit test

@TheLartians
Copy link
Member Author

After some testing I believe that the option should not be set by default, as even some of my own projects depend on installation of components defined only in outer dependencies. This might not be a best practice, but setting the flag by default could unintentionally break other projects as well. A smoother transition may be achieved by adding an optional flag that may be promoted to default sometime in the future.

@TheLartians TheLartians marked this pull request as ready for review February 16, 2021 10:19
@TheLartians TheLartians enabled auto-merge (squash) February 16, 2021 10:24
@TheLartians TheLartians linked an issue Feb 16, 2021 that may be closed by this pull request
@TheLartians TheLartians merged commit fd539b8 into master Feb 16, 2021
@TheLartians TheLartians deleted the exclude-from-all branch February 16, 2021 10:26
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.

Allowing EXCLUDE_FROM_ALL
1 participant