dunst-project / dunst

Lightweight and customizable notification daemon
https://dunst-project.org
Other
4.56k stars 340 forks source link

Rewrite github action #1312

Closed zappolowski closed 6 months ago

zappolowski commented 6 months ago

Yet another try of getting the pipelines to work. This continues/replaces #1303.

Notable changes:

TODO:

codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 65.06%. Comparing base (a2af4a8) to head (03f5861). Report is 6 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1312 +/- ## ========================================== - Coverage 65.52% 65.06% -0.46% ========================================== Files 48 48 Lines 7997 8173 +176 ========================================== + Hits 5240 5318 +78 - Misses 2757 2855 +98 ``` | [Flag](https://app.codecov.io/gh/dunst-project/dunst/pull/1312/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/dunst-project/dunst/pull/1312/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | `65.06% <ø> (-0.46%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bynect commented 6 months ago

Seems good to me, however I am not sure what changes from #1303 (?)

  • why generate coverage data for clang, when it's discarded anyhow (just gcc is uploaded); why upload coverage data for each distro? shouldn't one suffice?

I second that.

zappolowski commented 6 months ago

however I am not sure what changes from https://github.com/dunst-project/dunst/pull/1303 (?)

bynect commented 6 months ago

however I am not sure what changes from https://github.com/dunst-project/dunst/pull/1303 (?)

  • a different approach to fix this issue - instead of running as a different user, explicitly allow it using git config (as it's done in actions/checkout already)
  • alpine image is not excluded anymore
  • all tests are running - in #1303 DBus tests are deactivated

Thanks for the clarification. Is something still missing or will be able to fix ci by merging this one?

zappolowski commented 6 months ago

From my side I'm okay with merging, maybe @bebehei wants to take another look as he originally started working on this.

bebehei commented 6 months ago

I'll have alook tomorrow.

bynect commented 6 months ago

news?

zappolowski commented 6 months ago

My suggestion: merge it in the current state (if @bebehei has no strong objections) and get the CI up and running again. This way we can merge some other PRs which are currently blocked by the broken CI.

Then I can iron out the missing parts.

zappolowski commented 6 months ago

I've reworked coverage creation a bit. Now, it's just created when all tests pass, only for gcc and only for the main repository (hopefully, haven't tested it yet). I've chosen to run that step on the fedora image as this is usually quite up-to-date, not a rolling release distribution and provides lcov in the repositories.

bynect commented 5 months ago

i don't know why but coverage is failing (yesterday was fine though)

zappolowski commented 5 months ago

Yes, I think it's related to this issue.

I'll just update to v4 ... which should work as we're using fedora based images for the coverage reports (IIRC v4 had issues on older ubuntu based images).

bynect commented 5 months ago

Yes, I think it's related to this issue.

I'll just update to v4 ... which should work as we're using fedora based images for the coverage reports (IIRC v4 had issues on older ubuntu based images).

Thanks

Also, I wanted to make a minor version soon since a lot of commits piled up. I wrote an email to the other maintains but unfortunately I don't have your email. Do you have some prs still unfinished that you want to include?

zappolowski commented 5 months ago

You should be able to get my address from the commits (just look at the signature).

Other than that: