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

tests: Improve variable definition in tests/e2e scripts #346

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

GabyCT
Copy link
Contributor

@GabyCT GabyCT commented Jan 31, 2024

To have a better consistency across the test/e2e scripts this PR improves the variable definition by using local as a best practice.

Copy link
Contributor

@ldoktor ldoktor left a comment

Choose a reason for hiding this comment

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

Nice, it'd be even better to fix this issue across all shell scripts within repository (or at least directory). Are you interested in doing it (here or in a following PR), or do you want me to take care of that (I understand it's quite boring, but consistent :D)

@GabyCT
Copy link
Contributor Author

GabyCT commented Feb 2, 2024

@ldoktor let me do it

@GabyCT GabyCT changed the title tests: Improve variable definition in lib script tests: Improve variable definition in tests/e2e scripts Feb 6, 2024
@GabyCT
Copy link
Contributor Author

GabyCT commented Feb 6, 2024

@ldoktor @wainersm changes applied

@ldoktor
Copy link
Contributor

ldoktor commented Feb 7, 2024

Looks good although I spotted a few variables in operator.sh in wait_for_stabilization function, could you please include them as well? (that should be all, I hope :-D)

To have a better consistency across the test/e2e scripts this PR improves
the variable definition by using local as a best practice.

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
@GabyCT
Copy link
Contributor Author

GabyCT commented Feb 7, 2024

@ldoktor thanks for the feedback, sorry that I missed those

Copy link
Contributor

@ldoktor ldoktor left a comment

Choose a reason for hiding this comment

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

Thank you for bearing with my nitpicks :-)

@GabyCT
Copy link
Contributor Author

GabyCT commented Feb 7, 2024

Thank you for bearing with my nitpicks :-)

not worries, thanks for the feedback helps me to improve my work

Copy link
Member

@wainersm wainersm left a comment

Choose a reason for hiding this comment

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

Thanks @GabyCT !

@wainersm wainersm merged commit 016a7d4 into confidential-containers:main Feb 21, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants