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

fix: ignore deleted directories not part of a stack in change detection #287

Merged
merged 8 commits into from
Apr 1, 2022

Conversation

i4ki
Copy link
Contributor

@i4ki i4ki commented Mar 30, 2022

No description provided.

@i4ki i4ki requested a review from katcipis March 30, 2022 19:57
@i4ki i4ki self-assigned this Mar 30, 2022
@i4ki i4ki force-pushed the i4k-fix-change-detection-deleted-file branch from b9ac89e to 1036aa7 Compare March 31, 2022 08:05
@i4ki i4ki changed the title fix: ignore deleted files in change detection fix: ignore deleted directories not part of a stack in change detection Mar 31, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2022

Codecov Report

Merging #287 (4d72386) into main (77af5ff) will increase coverage by 0.08%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #287      +/-   ##
==========================================
+ Coverage   63.97%   64.05%   +0.08%     
==========================================
  Files          31       31              
  Lines        5984     5984              
==========================================
+ Hits         3828     3833       +5     
+ Misses       1944     1939       -5     
  Partials      212      212              
Flag Coverage Δ
tests 64.05% <0.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
hcl/hcl.go 86.66% <0.00%> (ø)
manager.go 63.23% <0.00%> (+0.24%) ⬆️
git/git.go 74.74% <0.00%> (+0.33%) ⬆️
test/sandbox/sandbox.go 85.87% <0.00%> (+0.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77af5ff...4d72386. Read the comment docs.

katcipis
katcipis previously approved these changes Mar 31, 2022
Copy link
Contributor

@katcipis katcipis left a comment

Choose a reason for hiding this comment

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

LGTM + some possible small improvements on the tests

assertRun(t, cli.listChangedStacks())
}

func TestListChangedDontIgnoreStackDeletedFilesInGitDiff(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func TestListChangedDontIgnoreStackDeletedFilesInGitDiff(t *testing.T) {
func TestListChangedDetectsDeletedStackSubdirFiles(t *testing.T) {

Comment on lines 227 to 228
git.Add(".")
git.Commit("all")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
git.Add(".")
git.Commit("all")
git.CommitAll("all")

})
}

func TestListChangedDontIgnoreStackDeletedDirectoriesInGitDiff(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func TestListChangedDontIgnoreStackDeletedDirectoriesInGitDiff(t *testing.T) {
func TestListChangedDetectsDeletedStackSubdir(t *testing.T) {

Comment on lines 252 to 253
git.Add(".")
git.Commit("all")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
git.Add(".")
git.Commit("all")
git.CommitAll("all")

Comment on lines 259 to 260
git.Add(".")
git.Commit("removed directory")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
git.Add(".")
git.Commit("removed directory")
git.CommitAll("removed directory")

Comment on lines 234 to 235
git.Add(".")
git.Commit("removed file")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
git.Add(".")
git.Commit("removed file")
git.CommitAll("removed file")

Comment on lines 212 to 213
git.Add(".")
git.Commit("removed stack")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
git.Add(".")
git.Commit("removed stack")
git.CommitAll("removed stack")

Comment on lines 205 to 206
git.Add(".")
git.Commit("all")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
git.Add(".")
git.Commit("all")
git.CommitAll("all")

katcipis
katcipis previously approved these changes Mar 31, 2022
Copy link
Contributor

@katcipis katcipis left a comment

Choose a reason for hiding this comment

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

LGTM + possible small improvements

git := s.Git()
git.CommitAll("all")
git.Push("main")
git.CheckoutNew("deleted-file")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
git.CheckoutNew("deleted-file")
git.CheckoutNew("deleted-dir")

@i4ki i4ki merged commit 5dd8b23 into main Apr 1, 2022
@i4ki i4ki deleted the i4k-fix-change-detection-deleted-file branch April 1, 2022 14:54
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.

3 participants