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

[App Check] Remove unused forward declaration #12167

Closed
wants to merge 1 commit into from

Conversation

ncooke3
Copy link
Member

@ncooke3 ncooke3 commented Dec 4, 2023

This line has existed within the public header for a few years. The line forward declares a protocol that (1) does not exist, and (2) is not referenced elsewhere within the header.

I don't think this is a breaking change because any client trying to reference the protocol from importing this header would need to provide an interface for the type themselves. But, I would appreciate feedback here on whether this sounds like a breaking change or not. And also if you think this needs a changelog entry.

#no-changelog

@ncooke3
Copy link
Member Author

ncooke3 commented Dec 4, 2023

Hi @sunmou99, the API diff report workflow seems to be having trouble with this PR. Could you please take a look?

@paulb777
Copy link
Member

paulb777 commented Dec 4, 2023

Looks like this was already done in #12067

@paulb777
Copy link
Member

paulb777 commented Dec 4, 2023

And maybe that's why the apidiff test is failing?

@andrewheard
Copy link
Contributor

Looks like this was already done in #12067

Good catch, @paulb777. Specifically, here: a832979#diff-a5e6bbe376196aed5e2c5abe55cc87825304549ef1c61d1ce5810aa4a2cb023d

@ncooke3
Copy link
Member Author

ncooke3 commented Dec 4, 2023

Ah, I missed that. Thanks y'all. I'm not sure even how I came across it. I think I followed a permalink to a dated commit and then came across the old version of the header.

@ncooke3 ncooke3 closed this Dec 4, 2023
@paulb777 paulb777 deleted the nc/unneeded-fwd-decl branch December 4, 2023 23:21
@firebase firebase locked and limited conversation to collaborators Jan 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants