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

avoid duplicated include files #9796

Closed
wants to merge 2 commits into from
Closed

Conversation

bagder
Copy link
Member

@bagder bagder commented Oct 24, 2022

No description provided.

@dfandrich
Copy link
Contributor

Looks fine to me, but maybe it's better to unconditionally include memdebug.h et. al. in md4/md5/sha256 so it's not forgotten in new backends or if existing backends are changed. Also, the regex would be better with "\s" instead of " " to catch tabs as well as spaces.

@bagder
Copy link
Member Author

bagder commented Oct 25, 2022

Looks fine to me, but maybe it's better to unconditionally include memdebug.h et. al. in md4/md5/sha256 so it's not forgotten in new backends or if existing backends are changed.

A good idea. But to do that properly we need to rearrange the code a bit so that all include files are included at the top and not distributed all over like now. Which also would make the files a little cleaner I think.

Also, the regex would be better with "\s" instead of " " to catch tabs as well as spaces.

Good point. Fixing.

@bagder
Copy link
Member Author

bagder commented Oct 25, 2022

I will wait with merging this until after the 7.86.0 release

@bagder bagder force-pushed the bagder/checksrc-multi-include branch from 741b88f to 5f90db5 Compare October 25, 2022 14:21
@bagder bagder closed this in 3678336 Oct 26, 2022
bagder added a commit that referenced this pull request Oct 26, 2022
@bagder bagder deleted the bagder/checksrc-multi-include branch October 26, 2022 09:28
@bagder bagder mentioned this pull request Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants