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

Remove pendingToken from public API. #2676

Merged
merged 1 commit into from Mar 29, 2019
Merged

Conversation

@Yue-Wang-Google
Copy link
Contributor

@Yue-Wang-Google Yue-Wang-Google commented Mar 29, 2019

It's added to public header by mistake.

Developers are not supposed to use it, and they currently have no way to use it.

…istake.

Developers are not supposed to use it, and they currently have no way to use it.
@Yue-Wang-Google Yue-Wang-Google changed the title Remove pendingToken from public API. It's added to public header by m… Remove pendingToken from public API. Mar 29, 2019
@Yue-Wang-Google
Copy link
Contributor Author

@Yue-Wang-Google Yue-Wang-Google commented Mar 29, 2019

Hey Morgan --- We accidentally added a private field to public API. Do you know what's the process to remove it?

Unlike last time (Microsoft/Yahoo provider ID), this time developer won't use this field at all. Even they grabbed this token, they can do nothing about it. It's a field of no use case at all. I was wondering in this case can we just remove this from the public header?

@Yue-Wang-Google Yue-Wang-Google requested a review from ulukaya Mar 29, 2019
@Yue-Wang-Google Yue-Wang-Google merged commit 76a6983 into master Mar 29, 2019
2 checks passed
@Yue-Wang-Google Yue-Wang-Google deleted the remove-pending-token branch Mar 29, 2019
@paulb777
Copy link
Member

@paulb777 paulb777 commented Mar 29, 2019

It may be ok to do this API change without a deprecation along with the major release update, but it should be cleared with @ryanwilson.

@paulb777 paulb777 added this to the Firebase 6 milestone Mar 29, 2019
@Yue-Wang-Google
Copy link
Contributor Author

@Yue-Wang-Google Yue-Wang-Google commented Mar 29, 2019

Hi @ryanwilson we accidentally left a private property in public header. Developer won't use it as there's absolutely no use case at all. I would consider this a bug. Is it ok to remove it in the next release?

@ryanwilson
Copy link
Member

@ryanwilson ryanwilson commented Apr 3, 2019

I think I'm okay with this but I'm not thrilled about it - can we have a discussion at some point soon about how this was released, and what steps are being taken in the future to avoid something like this happening? Having the very large PR (#2405) hurt the review-ability and introduced more than one issue with the public interface.

@Yue-Wang-Google
Copy link
Contributor Author

@Yue-Wang-Google Yue-Wang-Google commented Apr 3, 2019

I sent you an email discussing the issue. Let's continue the discussion internally.

Corrob added a commit that referenced this issue Apr 25, 2019
…istake. (#2676)

Developers are not supposed to use it, and they currently have no way to use it.
@firebase firebase locked and limited conversation to collaborators Oct 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants