Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Let user specify the firebase app name #716

Merged
merged 12 commits into from
Jul 1, 2024

Conversation

MvRemmerden
Copy link
Contributor

No description provided.

Copy link

vercel bot commented Jun 27, 2024

@MvRemmerden is attempting to deploy a commit to the Gladly Team on Vercel.

A member of the Team first needs to authorize it.

@MvRemmerden
Copy link
Contributor Author

@kmjennison I think this PR might be ready, would you mind approving it to be deployed on Vercel so we can see whether any of the checks fail? 🙂

Copy link

vercel bot commented Jun 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nfa-example ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 1, 2024 2:25pm

Copy link
Contributor

@kmjennison kmjennison left a comment

Choose a reason for hiding this comment

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

@MvRemmerden Thanks for this! The approach looks good. Could you investigate the failing tests?

@MvRemmerden
Copy link
Contributor Author

@kmjennison I might need some help here. It looks like the problem is due to this change here: https://github.com/gladly-team/next-firebase-auth/pull/716/files#diff-40051885c173eb84152e58b7e9f3cbac7bd1e5d9de2520a1bd2a4253b4a6b9a0R108

I'm not sure if we just need to adapt the test to ensure that the app is initialized before, or if there is a bug here in the actual code.

Would you mind having a look yourself?

@MvRemmerden
Copy link
Contributor Author

Update: I think I got it working, it might have just been missing mocked configs in the tests, everything is passing now ✅

Copy link
Contributor

@kmjennison kmjennison left a comment

Choose a reason for hiding this comment

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

One change needed and otherwise looks good.

src/configTypes.ts Outdated Show resolved Hide resolved
Co-authored-by: Kevin Jennison <kevin.jennison1@gmail.com>
@MvRemmerden
Copy link
Contributor Author

@kmjennison Awesome, committed, back to you 🙂

Copy link
Contributor

@kmjennison kmjennison left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your work on this!

@kmjennison kmjennison merged commit ebc2b2b into gladly-team:v1.x Jul 1, 2024
3 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants