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.

Sign up for GitHub

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

Jump to bottom

Added "T-Shirt and Numbers" option #88

Merged
merged 3 commits into from
Nov 9, 2023
Merged

Added "T-Shirt and Numbers" option #88

merged 3 commits into from
Nov 9, 2023

Conversation

moritzluedtke
Copy link
Contributor

No description provided.

@moritzluedtke moritzluedtke mentioned this pull request Nov 7, 2023
Closed
hellomuthu23
hellomuthu23 reviewed Nov 7, 2023
View reviewed changes
Copy link
Owner

@hellomuthu23 hellomuthu23 left a comment

Choose a reason for hiding this comment

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

Looks good @moritzluedtke. This option can be certainly useful for few users. Just need to add unit tests and test it in preview env.

@moritzluedtke
Copy link
Contributor Author

Thanks for taking a look. I'll try to fix them soon.

@moritzluedtke
Copy link
Contributor Author

moritzluedtke commented Nov 8, 2023 •
edited
Loading

@hellomuthu23 I added some tests (should've done that in the first place) and fixed the broken one.

@moritzluedtke
Copy link
Contributor Author

I'd need some support with the failing workflow since I have no experience with your setup and Firebase. Looks like a config problem, though.

@hellomuthu23
Copy link
Owner

I'd need some support with the failing workflow since I have no experience with your setup and Firebase. Looks like a config problem, though.

yes, its a config problem. When the branch is not from actual source and the user doesn't have access to secrets.

@hellomuthu23 hellomuthu23 merged commit 62ff1f8 into hellomuthu23:master Nov 9, 2023
1 of 2 checks passed
@moritzluedtke
Copy link
Contributor Author

Ah, I see. Thanks for the explanation and the quick merging of my PR!

@hellomuthu23
Copy link
Owner

Ah, I see. Thanks for the explanation and the quick merging of my PR!

no problem, thanks a lot for your contribution. Highly appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hellomuthu23 hellomuthu23 hellomuthu23 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@moritzluedtke @hellomuthu23