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

Update spdlog so it works with recent musls #2921

Merged
merged 1 commit into from
Nov 5, 2024
Merged

Conversation

cataphract
Copy link
Contributor

gabime/spdlog@287a00d

Description

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@cataphract cataphract requested a review from a team as a code owner November 4, 2024 17:55
@bwoebi
Copy link
Collaborator

bwoebi commented Nov 4, 2024

That probably fixed that particular issue, but now a load of new errors appear, probably due to the other changes between these commits...

And please rebase the commit on top of a newer master, CI fails too much too :-)

@cataphract cataphract force-pushed the glopes/update-spdlog branch from 970e88c to 985c5c3 Compare November 4, 2024 18:42
@cataphract cataphract force-pushed the glopes/update-spdlog branch from 985c5c3 to f34823a Compare November 4, 2024 18:44
@cataphract
Copy link
Contributor Author

Rebase and fixed the errors related to the new spdlog, at least in the main helper binary.

@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.39%. Comparing base (352d3ba) to head (f34823a).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2921   +/-   ##
=========================================
  Coverage     72.39%   72.39%           
  Complexity     2526     2526           
=========================================
  Files           135      135           
  Lines         14400    14400           
  Branches        989      989           
=========================================
  Hits          10425    10425           
  Misses         3433     3433           
  Partials        542      542           
Flag Coverage Δ
appsec-extension 68.36% <ø> (ø)
tracer-php 73.96% <ø> (ø)

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


Continue to review full report in Codecov by Sentry.

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

@pr-commenter
Copy link

pr-commenter bot commented Nov 4, 2024

Benchmarks [ appsec ]

Benchmark execution time: 2024-11-04 19:18:36

Comparing candidate commit f34823a in PR branch glopes/update-spdlog with baseline commit 352d3ba in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics.

Copy link
Collaborator

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

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

Looks good, thanks Gustavo.

Copy link
Contributor

@Anilm3 Anilm3 left a comment

Choose a reason for hiding this comment

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

lgtm

@Anilm3
Copy link
Contributor

Anilm3 commented Nov 5, 2024

The integration tests are failing because they're expecting an image for PHP 8.4 which doesn't exist at the moment, you can disable the 8.4 test for now and I'll add this to the backlog.

@bwoebi bwoebi merged commit 1b40137 into master Nov 5, 2024
712 of 743 checks passed
@bwoebi bwoebi deleted the glopes/update-spdlog branch November 5, 2024 11:51
@github-actions github-actions bot added this to the 1.5.0 milestone Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants