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

Added Clerk Auth #180

Closed
wants to merge 2 commits into from
Closed

Added Clerk Auth #180

wants to merge 2 commits into from

Conversation

AaronMBMorse
Copy link

Added clerk authentication as an option.

@nisrulz
Copy link
Owner

nisrulz commented Aug 9, 2023

Hi @AaronMBMorse , thank you for your PR. Everything looks good to me code wise, however your PR came from master branch which is an issue. You see, my Github Action workflow skips the job to publish a preview version of the webapp if the job was run on master branch.

It would be nice if you branched off your master and then sent the PR.

Additionally for future, I would suggest you to do the same flow as sending PR from master is not advised. Many open source devs create CI scripts that would skip a job if run on a branch that has master in it.

You can close this in favour of a new branch PR you create.

@nisrulz
Copy link
Owner

nisrulz commented Aug 10, 2023

Closing this PR in favour of the one I created and merged.

Thank you raising this PR and your contribution.

@nisrulz nisrulz closed this Aug 10, 2023
@AaronMBMorse
Copy link
Author

Thanks for the recommendation. I'll plan on that for the next PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants