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

ci: reworked windows builds #737

Merged
merged 1 commit into from
Dec 6, 2021
Merged

ci: reworked windows builds #737

merged 1 commit into from
Dec 6, 2021

Conversation

sergiud
Copy link
Collaborator

@sergiud sergiud commented Nov 9, 2021

Closes #735

@sergiud sergiud added the bug label Nov 9, 2021
@sergiud sergiud added this to the 0.6 milestone Nov 9, 2021
@google-cla google-cla bot added the cla: yes label Nov 9, 2021
@coveralls
Copy link
Collaborator

coveralls commented Nov 9, 2021

Coverage Status

Coverage decreased (-4.8%) to 71.852% when pulling 840b520 on ci-win32-builds into 503e3de on master.

@sergiud sergiud force-pushed the ci-win32-builds branch 24 times, most recently from ae8ea61 to f2e8e30 Compare November 13, 2021 11:38
@sergiud sergiud force-pushed the ci-win32-builds branch 2 times, most recently from 921d437 to cbaa441 Compare November 19, 2021 20:26
@sergiud sergiud force-pushed the ci-win32-builds branch 6 times, most recently from 4880c64 to 5407543 Compare November 19, 2021 22:59
@sergiud sergiud force-pushed the ci-win32-builds branch 5 times, most recently from 98ae205 to 840b520 Compare December 3, 2021 19:34
@sergiud
Copy link
Collaborator Author

sergiud commented Dec 6, 2021

@drigz I've been trying to fix merging of multiple coverage reports (mainly MinGW and Linux) over the the past weeks. However, this turned out to be very tricky due to the way both Coveralls and Github workflows work. Since the Github workflows are defined in separate files, merging the reports cannot be done automatically and requires additional logic which is not straightforward to implement. Additionally, Coveralls seems to have problems parsing MinGW coverage reports (a 0% coverage is always reported).

At this point, I believe switching to Codecov is a better solution in the long-term.

@drigz
Copy link
Member

drigz commented Dec 6, 2021

LGTM. Let me know if you need any action from me to enable Codecov.

@sergiud
Copy link
Collaborator Author

sergiud commented Dec 6, 2021

Thanks. We would need a CODECOV_TOKEN secret with Codecov token assigned to it as detailed here. Coveralls bot access to repo can be revoked.

@drigz
Copy link
Member

drigz commented Dec 6, 2021

Where would we store the CODECOV_TOKEN? Assuming we wouldn't be manually uploading coverage data, it's not clear how it should be used. It also says "Note: Token not required for public repositories uploading from Travis, CircleCI, AppVeyor, Azure Pipelines or GitHub Actions." - does this apply to us?

@codecov-commenter
Copy link

codecov-commenter commented Dec 6, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@503e3de). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #737   +/-   ##
=========================================
  Coverage          ?   71.38%           
=========================================
  Files             ?       17           
  Lines             ?     3208           
  Branches          ?        0           
=========================================
  Hits              ?     2290           
  Misses            ?      918           
  Partials          ?        0           

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 503e3de...536df2a. Read the comment docs.

@sergiud
Copy link
Collaborator Author

sergiud commented Dec 6, 2021

I obviously ignored that note, thanks. The token does not seem to be necessary.

@sergiud sergiud force-pushed the ci-win32-builds branch 9 times, most recently from 44937ae to 82927ee Compare December 6, 2021 16:06
@sergiud sergiud merged commit f4dd77a into master Dec 6, 2021
@sergiud sergiud deleted the ci-win32-builds branch December 6, 2021 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Two tests failed after successful compilation by msvc on the windows x64 platform
4 participants