s3deploy icon indicating copy to clipboard operation
s3deploy copied to clipboard

Fix osFile.ACL bug if route is nil

Open satotake opened this issue 2 years ago • 5 comments

Fixes #283

satotake avatar Aug 24 '22 20:08 satotake

Codecov Report

Merging #284 (8d0c0dd) into master (73f2d9d) will increase coverage by 0.26%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   70.13%   70.39%   +0.26%     
==========================================
  Files           9        9              
  Lines         683      689       +6     
==========================================
+ Hits          479      485       +6     
  Misses        174      174              
  Partials       30       30              
Impacted Files Coverage Δ
lib/config.go 96.92% <100.00%> (+0.37%) :arrow_up:
lib/deployer.go 81.50% <100.00%> (-0.52%) :arrow_down:
lib/files.go 80.58% <100.00%> (+0.78%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Aug 24 '22 20:08 codecov-commenter

Thanks for this.

I do however suggests that we take another approach fixing this, which I suspect will make this less fragile in the future:

  • We say that every file has a route, so if a file doesn't get a match in the routes set, just create one but some non nil header map and a default acl.

bep avatar Aug 24 '22 20:08 bep

I'm reverting the failing commit now, I suspect that people pull in the latest via go get, so it's better to get a fix out there sooner rather than later.

We can talk about this later, it's later here in Norway.

bep avatar Aug 24 '22 21:08 bep

I'm reverting the failing commit now, I suspect that people pull in the latest via go get, so it's better to get a fix out there sooner rather than later.

Agreed with it. thank you.

We say that every file has a route, so if a file doesn't get a match in the routes set, just create one but some non nil header map and a default acl.

ok.

satotake avatar Aug 25 '22 09:08 satotake

@bep done! Could you check it?

satotake avatar Aug 25 '22 11:08 satotake