-
Notifications
You must be signed in to change notification settings - Fork 5
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
AAE-11722 ease up reportportal integration #200
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.
Unless you're explicitly exploiting the Parameter Expansion / Word Splitting feature, it's advisable to always double-quote variables to prevent possible side effects.
Hence, no matter if you use $FOO_BAR
or ${FOO_BAR}
, these should always be enclosed in ", like in "$FOO_BAR"
and "${FOO_BAR}"
expected=$(cat << BATS | ||
enabled=true | ||
key=my launch prefix with spaces-weird event-3665876492 | ||
url=https://rpserver:8080 with spaces too/ui/#my project with spaces/launches/all |
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 think it would be good to have the reposerver as env variable
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.
It is the default value for these helper actions, but you can define as an env variable from the caller if it differs
This PR adds two new actions to help with reportportal integration.
It also allows appending a custom message to the default one when sending a slack notification (useful in reportportal usage situations where we want to add information to link to reportportal launches).
There's heavy bash usage (and I'm thankful for bats) but I'm not fluent in bash: all feedback on bash commands is very welcome.
I will link several dependent PRs referencing this PR soon.