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

Return exit code 1 when infrastructure not in sync #156

Merged
merged 1 commit into from
Jan 28, 2021
Merged

Conversation

eliecharra
Copy link
Contributor

Q A
πŸ› Bug fix? no
πŸš€ New feature? yes
⚠ Deprecations? no
❌ BC Break yes
πŸ”— Related issues #...
❓ Documentation no

@eliecharra eliecharra added the kind/enhancement New feature or improvement label Jan 27, 2021
@eliecharra eliecharra requested a review from a team January 27, 2021 18:13
@codecov
Copy link

codecov bot commented Jan 27, 2021

Codecov Report

Merging #156 (c4c5499) into main (c77d601) will decrease coverage by 0.11%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #156      +/-   ##
==========================================
- Coverage   67.78%   67.67%   -0.12%     
==========================================
  Files         186      187       +1     
  Lines        4200     4207       +7     
==========================================
  Hits         2847     2847              
- Misses       1045     1052       +7     
  Partials      308      308              
Impacted Files Coverage Ξ”
main.go 5.26% <0.00%> (-0.30%) ⬇️
pkg/cmd/driftctl.go 73.58% <ΓΈ> (ΓΈ)
pkg/cmd/errors/scan.go 0.00% <0.00%> (ΓΈ)
pkg/cmd/scan.go 80.00% <0.00%> (-1.99%) ⬇️

moadibfr
moadibfr previously approved these changes Jan 28, 2021
Copy link
Contributor

@moadibfr moadibfr left a comment

Choose a reason for hiding this comment

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

LGTM
I think it's good to have error in the package that return them. It's more or less what I did in my PR but with an interface to prepare same error for different providers.

@eliecharra
Copy link
Contributor Author

@moadibfr You have approved but do you want me to move related error in package they return them ?

@eliecharra
Copy link
Contributor Author

@moadibfr Moved error in package it is triggered

@eliecharra eliecharra merged commit 2cc4728 into main Jan 28, 2021
@eliecharra eliecharra deleted the exit_code branch January 28, 2021 14:00
@eliecharra eliecharra added this to the v0.4.0 milestone Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants