codecov / codecov-exe

.exe report uploader for Codecov https://codecov.io

MIT License

25 stars 21 forks source link

Remove Public Read header from presigned PUT #105

Closed hootener closed 4 years ago

hootener commented 4 years ago

This seems to be failing in CI with:

Restoring NuGet package Cake.0.32.1.
  GET https://api.nuget.org/v3-flatcontainer/cake/0.32.1/cake.0.32.1.nupkg
  OK https://api.nuget.org/v3-flatcontainer/cake/0.32.1/cake.0.32.1.nupkg 12ms
Installing Cake 0.32.1.
Adding package 'Cake.0.32.1' to folder '/home/appveyor/projects/codecov-exe/tools'
Added package 'Cake.0.32.1' to folder '/home/appveyor/projects/codecov-exe/tools'
Error: The assembly name is invalid.
Command exited with code 1
Build failed

And I just don't possess the knowledge of this application to make a fix here. Please let me know if I can help, @AdmiringWorm and @larzw

AdmiringWorm commented 4 years ago

@hootener thank you for opening this PR. I was not aware that the storage location for S3 had been changed to something else as thus not needing this request header anymore.

Regarding the CI error, this is something I have been struggling to figure out myself as well, but so far have been unsuccessful in finding out why the failures happen. The build only fails on Unix, and I believe there may be an incompatibility with the mono version on the CI server compared to earlier versions.

I may need to rewrite the build scripts to before I can merge in this PR.

hootener commented 4 years ago

I may need to rewrite the build scripts to before I can merge in this PR.

@AdmiringWorm Got it. Please let me know if there's anything else I need to do with respect to this PR or to unblock this process. Thanks!

sn1020 commented 4 years ago

@hootener Might be helpful to add a readme update to this PR, similar to https://github.com/codecov/codecov-node/pull/181?

hootener commented 4 years ago

Done @sn1020 .

@AdmiringWorm I see this CI on this is passing now. Is it good to merge and release version 1.11.0?

AdmiringWorm commented 4 years ago

@hootener Unfortunately not yet, still working on a fixed build script. The check succeeded because only the GitHub action ran (which unfortunately do not do any integration test yet), not the appveyor one which is the main one (it doesn't run on pull requests unfortunately due to missing the pull request event hook).

So I need to be sure that it works before merging this one in.

codecov[bot] commented 4 years ago

Codecov Report

Merging #105 into develop will not change coverage. The diff coverage is 0.00%.

@@           Coverage Diff            @@
##           develop     #105   +/-   ##
========================================
  Coverage    66.94%   66.94%           
========================================
  Files           36       36           
  Lines          947      947           
  Branches       120      120           
========================================
  Hits           634      634           
  Misses         313      313

Impacted Files	Coverage Δ
Source/Codecov/Upload/CodecovUploader.cs	`10.90% <0.00%> (ø)`

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 89b457b...c0c41ba. Read the comment docs.

AdmiringWorm commented 4 years ago

@hootener your changes have been merged, thanks for your contribution 👍

thomasrockhu commented 4 years ago

Thanks @AdmiringWorm!

github-actions[bot] commented 4 years ago

:tada: This issue has been resolved in version 1.11.0 :tada:

The release is available on:

Your friendly GitReleaseManager bot :package::rocket: