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
feat: Sentry Create Deploy Action #83
feat: Sentry Create Deploy Action #83
Conversation
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.
I'd trust @philipphofmann and/or @HazAT's review but just some suggestions from my end:
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.
Thanks for taking care of this. I added a few comments.
@philipphofmann @bruno-garcia Thank you for your feedback, i have incorporated everything. Also, i have noticed that tests are failing for the upload action when i run them locally, which should not have been affected by this PR. Is this something known?
|
Also:
|
No this should be fixed. Do you maybe have time to open up another PR to fix these broken tests? |
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.
Thanks, LGTM.
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.
LGTM
thanks @denrase you may add Ruby to your CV :D
@marandaneto Did collect all Pokémon as a kid, only natural to continue with programming languages in adulthood. ^^ |
Will look into it! |
Overview
sentry_create_deploy
action.url
parameter of thesentry-cli
command is nameddeploy_url
in the action, as it collides with another param.Testing
rspec
tests similar to other actions, mainly testing the parameters and how they translate tosentry-cli
parameters.senty.io
though, is this a permissions/account "issue"?Relates to #82