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.

Sign up for GitHub

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

Jump to bottom

Add source in JSON output #906

Merged
merged 1 commit into from Aug 4, 2021
Merged

Add source in JSON output #906

merged 1 commit into from Aug 4, 2021

Conversation

eliecharra
Copy link
Contributor

Q A
πŸ› Bug fix? no
πŸš€ New feature? yes
⚠ Deprecations? no
❌ BC Break no
πŸ”— Related issues #874
❓ Documentation snyk/driftctl-docs#90

@eliecharra eliecharra requested a review from a team as a code owner August 3, 2021 08:33
@codecov
Copy link

codecov bot commented Aug 3, 2021 •
edited

Codecov Report

Merging #906 (c1ea0a8) into main (75cd450) will decrease coverage by 0.00%.
The diff coverage is 81.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #906      +/-   ##
==========================================
- Coverage   81.70%   81.70%   -0.01%     
==========================================
  Files         233      233              
  Lines        7588     7598      +10     
==========================================
+ Hits         6200     6208       +8     
- Misses       1161     1163       +2     
  Partials      227      227
Impacted Files Coverage Ξ”
pkg/resource/resource.go 76.30% <81.81%> (+0.22%) ⬆️

@eliecharra eliecharra added this to the v0.14 milestone Aug 3, 2021
@eliecharra eliecharra added this to Review in driftctl Aug 3, 2021
lotoussa
lotoussa previously approved these changes Aug 3, 2021
View reviewed changes
Copy link
Contributor

@lotoussa lotoussa left a comment

Choose a reason for hiding this comment

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

lgtm

wbeuil
wbeuil previously approved these changes Aug 4, 2021
View reviewed changes
Copy link
Contributor

@wbeuil wbeuil left a comment

Choose a reason for hiding this comment

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

Wonder where we still use FakeResource and FakeResourceStringer and whether they still make sense or not.

sundowndev
sundowndev reviewed Aug 4, 2021
View reviewed changes
Copy link
Contributor

@sundowndev sundowndev left a comment •
edited

Choose a reason for hiding this comment

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

Tried locally, works fine

branch needs rebase

@eliecharra eliecharra dismissed stale reviews from wbeuil and lotoussa via c1ea0a8 August 4, 2021 12:29
@eliecharra
Copy link
Contributor Author

Wonder where we still use FakeResource and FakeResourceStringer and whether they still make sense or not.

They definitively not make sense anymore, we should remove them when we'll do the Resource interface removal refactoring.
I just wanted to avoid any big impact for this PR

@sundowndev sundowndev merged commit 45b9af8 into main Aug 4, 2021
driftctl automation moved this from Review to Done Aug 4, 2021
@sundowndev sundowndev deleted the source_json_output branch August 4, 2021 12:36
@sundowndev sundowndev added the kind/maintenance Refactoring or changes to the workspace label Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sundowndev sundowndev sundowndev approved these changes

@wbeuil wbeuil wbeuil left review comments

@lotoussa lotoussa lotoussa left review comments

Assignees
No one assigned
Labels
kind/maintenance Refactoring or changes to the workspace
Projects
None yet
Milestone
v0.14
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@eliecharra @wbeuil @sundowndev @lotoussa