-
Notifications
You must be signed in to change notification settings - Fork 62
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
Support for custom deploy user (#54) #55
Conversation
@enriikke let me know if I can do something to get this PR merged. I'm happy to change or add anything you're unhappy with with the current changeset! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fantastic @lukasbach! Thank you so much for taking the time to write up a PR and my apologies for taking long to reply. ✨ ❤️
I added some suggestions to your PR and also there are a couple of things I would like to see to get it merged:
- Let's update the README to include the new inputs like we do for the existing inputs.
- It's a bit tricky to write tests for CLI tools like this one but it would be great to include a few to make sure we call
git config ...
with the right thing based not he inputs.
Hi @enriikke. Thanks for your suggestions, I've added them to the changeset. I've documented the inputs in the readme, added test cases for the arguments, removed the sha attribute from the commit message and formatted as you suggested. Feel free to let me know if there is something else I can do. Also, I had issues using the nullish coalescing operator that you suggested ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Thank you @lukasbach ✨
Cool, happy that I could help :) |
Adds the features requested in #54
Let me know if I should add or change anything. I'd be happy if the feature would be merged!