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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove some include directives for unused headers #1849

Merged
merged 1 commit into from Nov 21, 2021

Conversation

vittorioromeo
Copy link
Member

Thanks a lot for making a contribution to SFML! 馃檪

Before you create the pull request, we ask you to check the follow boxes. (For small changes not everything needs to ticked, but the more the better!)

  • Has this change been discussed on the forum or in an issue before?
  • Does the code follow the SFML Code Style Guide?
  • Have you provided some example/test code for your changes?
  • If you have additional steps which need to be performed list them as tasks!

Description

Self-explanatory.

Tasks

  • Tested on Linux
  • Tested on Windows
  • Tested on macOS
  • Tested on iOS
  • Tested on Android

How to test this PR?

CI will be good enough.

Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Nice, thanks! 馃檪

Out of curiosity, did you use tooling to detect unused headers?

@eXpl0it3r eXpl0it3r added this to Discussion in SFML 2.6.0 via automation Nov 21, 2021
@eXpl0it3r eXpl0it3r added this to the 2.6 milestone Nov 21, 2021
@eXpl0it3r eXpl0it3r moved this from Discussion to Ready in SFML 2.6.0 Nov 21, 2021
@vittorioromeo
Copy link
Member Author

Nice, thanks! 馃檪

Out of curiosity, did you use tooling to detect unused headers?

Yes, but not in the way you think. I was actually benchmarking SFML's compilation time via ClangBuildAnalyzer. The biggest bottleneck, by far, is sf::Utf, followed by sf::String. I was surprised to see that sf::String was being included many times, then I did some manual searches and found unnecessary includes.

We could try out this next: https://github.com/include-what-you-use/include-what-you-use

@eXpl0it3r eXpl0it3r merged commit dc88cbd into SFML:master Nov 21, 2021
SFML 2.6.0 automation moved this from Ready to Done Nov 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
SFML 2.6.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants