Skip to content

Conversation

@EfratIsrael
Copy link
Contributor

No description provided.

@MarshalX MarshalX changed the title CM-25773- initial commit CM-25773 - Support TF Plan scans Aug 1, 2023
Copy link
Contributor

@doratias18 doratias18 left a comment

Choose a reason for hiding this comment

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

Add more tests

@doratias18 doratias18 marked this pull request as ready for review August 1, 2023 17:54
@doratias18
Copy link
Contributor

@artem-fedorov @MichalBor @MarshalX @avishaiamiel please review

@doratias18 doratias18 requested a review from avishaiamiel August 1, 2023 17:55
Copy link
Contributor

@MichalBor MichalBor left a comment

Choose a reason for hiding this comment

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

Great job! 👏🏻

Copy link
Contributor

@MarshalX MarshalX left a comment

Choose a reason for hiding this comment

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

well done!

@MarshalX
Copy link
Contributor

MarshalX commented Aug 4, 2023

@EfratIsrael pls re-request review when you will be ready

@doratias18 doratias18 requested a review from MichalBor August 4, 2023 11:09
doratias18
doratias18 previously approved these changes Aug 4, 2023
Copy link
Contributor

@doratias18 doratias18 left a comment

Choose a reason for hiding this comment

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

Looks very good now. Thanks for your hard work.

@doratias18 doratias18 requested a review from MarshalX August 4, 2023 11:10
Copy link
Contributor

@MarshalX MarshalX left a comment

Choose a reason for hiding this comment

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

good structure of test files! i like that!

btw do we expect that tf plans files could be in the directory with another IaC related files? "scan path" will scan everything related to IaC.

im asking because i wonder that our user-firendly exception doesn't show the path to invalid tf plan file. will it be a problem for the user to find it in the provided path with many subfolders?

MichalBor
MichalBor previously approved these changes Aug 6, 2023
Copy link
Contributor

@MichalBor MichalBor left a comment

Choose a reason for hiding this comment

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

Amazing ⭐
please wait for Ilya approval

doratias18
doratias18 previously approved these changes Aug 6, 2023
@EfratIsrael EfratIsrael dismissed stale reviews from doratias18 and MichalBor via 1e1ac2d August 6, 2023 15:13
Copy link
Contributor

@MarshalX MarshalX left a comment

Choose a reason for hiding this comment

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

awesome!

custom_exceptions.TfplanKeyError: CliError(
soft_fail=True,
code='key_error',
message=f'\n{e!s}'
Copy link
Contributor

Choose a reason for hiding this comment

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

does !s required here?

image

Copy link
Contributor

@MarshalX MarshalX left a comment

Choose a reason for hiding this comment

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

rly hard work!

@EfratIsrael EfratIsrael merged commit 4c04a32 into main Aug 7, 2023
@EfratIsrael EfratIsrael deleted the CM-25773-support-tfplan-scan branch August 7, 2023 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants