Skip to content

Conversation

@rahulpatidar0191
Copy link
Member

  • Added a condition to run health test on a PRD docker image
  • Added a parameter so that all the tests can also run on PRD test image that includes dev dependencies
  • Ref:

@rahulpatidar0191 rahulpatidar0191 added the Type: Enhancement New feature or request label Dec 4, 2023
@rahulpatidar0191 rahulpatidar0191 self-assigned this Dec 4, 2023
Comment on lines 18 to 19
if [[ -n $DEV_ARG ]]
then
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: A better way to do this is to exit early. Instead of having to scroll down to check what happens after this if statement, I then know that everything else below is for DEV only since otherwise we exit the script entirely.
(Check the code below, my bash knowledge is rusty.

Suggested change
if [[ -n $DEV_ARG ]]
then
if [[ -z $DEV_ARG ]]
then
exit 0

Copy link
Member Author

Choose a reason for hiding this comment

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

It works

if [[ -z $DEV_ARG ]]
then
exit 0
else
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary!
Get rid of it and the diff between this PR and main will be a lot smaller too once you revert the indentation of the lines below!

Suggested change
else

Copy link
Contributor

@hillairet hillairet left a comment

Choose a reason for hiding this comment

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

Sweet!

Co-authored-by: Anthony Hillairet <anthony.hillairet@gmail.com>
@rahulpatidar0191 rahulpatidar0191 merged commit 7d7db99 into main Dec 5, 2023
@rahulpatidar0191 rahulpatidar0191 deleted the feature/tests-on-prd-img branch December 5, 2023 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants