Skip to content

Change it so that the header files are directly included#1865

Open
AustinBenoit wants to merge 2 commits into
mainfrom
direct-include
Open

Change it so that the header files are directly included#1865
AustinBenoit wants to merge 2 commits into
mainfrom
direct-include

Conversation

@AustinBenoit
Copy link
Copy Markdown
Contributor

Description

Provide details of the change, and generalize the change in the PR title above.

Change it so that the header files are directly included


Testing

Describe how you've tested these changes. Link any manually triggered Integration tests or CPP binary SDK Packaging Github Action workflows, if applicable.

Run all integration tests in github


Type of Change

Place an x the applicable box:

  • Bug fix. Add the issue # below if applicable.
  • New feature. A non-breaking change which adds functionality.
  • Other, such as a build process or documentation change.

Notes

  • Bug fixes and feature changes require an update to the Release Notes section of release_build_files/readme.md.
  • Read the contribution guidelines CONTRIBUTING.md.
  • Changes to the public API require an internal API review. If you'd like to help us make Firebase APIs better, please propose your change in a feature request so that we can discuss it together.

Remove the cmake injection and just include the header files as needed
@AustinBenoit AustinBenoit added the skip-release-notes Skip release notes check label Jun 3, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes the automatic global inclusion of headers like assert.h, string.h, and stdint.h from CMakeLists.txt files across various modules, replacing them with explicit #include statements in the individual source and header files. The reviewer feedback recommends two main improvements: first, reordering includes so that companion headers are always included first (consistent with the Google C++ Style Guide); second, preferring C++ standard library headers (such as , , and ) over their C-style counterparts (like <assert.h>, <stdint.h>, and <string.h>) to ensure consistency and proper namespace scoping.

Comment thread app/rest/controller_curl.cc Outdated
Comment thread app/src/app_android.cc Outdated
Comment thread database/src/common/data_snapshot.cc Outdated
Comment thread firestore/src/android/aggregate_query_android.cc Outdated
Comment thread functions/src/android/callable_reference_android.cc Outdated
Comment thread app/rest/response_json.h
Comment thread app/src/util_android.h Outdated
Comment thread firestore/src/main/field_value_main.h
Comment thread storage/src/include/firebase/storage/metadata.h
@AustinBenoit AustinBenoit force-pushed the direct-include branch 2 times, most recently from 0461c0e to d644d53 Compare June 3, 2026 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-release-notes Skip release notes check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant