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 supported versions workflow #4326

Merged
merged 11 commits into from
Feb 3, 2025

Conversation

quinna-h
Copy link
Contributor

@quinna-h quinna-h commented Jan 28, 2025

What does this PR do?

Makes the following changes:

  • Consolidate the logic from the generate_table_versions.rb script into the find_gem_version_bounds.rb script
  • Update the description in the auto-generated PR body and in the generated markdown table to make the purpose of the file more clear

Motivation:

Addition to #4210

Change log entry
No.

Additional Notes:

How to test the change?
See the generated PR here: #4236

Copy link

github-actions bot commented Jan 28, 2025

Thank you for updating Change log entry section 👏

Visited at: 2025-01-29 14:55:57 UTC

@github-actions github-actions bot added the dev/github Github repository maintenance and automation label Jan 28, 2025
@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Jan 28, 2025

Datadog Report

Branch report: quinna.halim/update-supported-versions-script
Commit report: 17db220
Test service: dd-trace-rb

✅ 0 Failed, 22115 Passed, 1476 Skipped, 5m 16.66s Total Time
⌛ 4 Performance Regressions

⌛ Performance Regressions vs Default Branch (4)

  • Rails integration tests for an application with a basic route behaves like appsec standalone billing with upstream appsec propagation without attack and trace is propagated as is from 2 sampling priority behaves like a request with propagated headers is expected to include "_dd.p.appsec=1" - rspec 4.43s (+3.79s, +592%) - Details
  • Rails integration tests for an application with a basic route behaves like appsec standalone billing with upstream appsec propagation without attack and trace is propagated as is from 1 sampling priority behaves like a trace with ASM Standalone tags is expected to eq 1212121212121212121 - rspec 4.65s (+4s, +618%) - Details
  • Rails integration tests for an application with a basic route behaves like appsec standalone billing with upstream appsec propagation without attack and trace is propagated as is from 2 sampling priority behaves like a trace sent to agent with Datadog-Client-Computed-Stats header is expected to equal true - rspec 3.68s (+3.05s, +485%) - Details
  • Rails integration tests for an application with a basic route behaves like appsec standalone billing with upstream appsec propagation without attack and trace is propagated as is from 1 sampling priority behaves like a trace sent to agent with Datadog-Client-Computed-Stats header is expected to equal true - rspec 4.68s (+4.05s, +641%) - Details

@pr-commenter
Copy link

pr-commenter bot commented Jan 28, 2025

Benchmarks

Benchmark execution time: 2025-01-29 20:19:56

Comparing candidate commit 17db220 in PR branch quinna.halim/update-supported-versions-script with baseline commit 38023b1 in branch master.

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

scenario:tracing - Propagation - Trace Context

  • 🟥 throughput [-4461.349op/s; -4362.247op/s] or [-11.716%; -11.455%]

@codecov-commenter
Copy link

codecov-commenter commented Jan 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.72%. Comparing base (04efab7) to head (81f1a59).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4326      +/-   ##
==========================================
- Coverage   97.72%   97.72%   -0.01%     
==========================================
  Files        1368     1368              
  Lines       82999    82998       -1     
  Branches     4220     4220              
==========================================
- Hits        81113    81107       -6     
- Misses       1886     1891       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marcotc
Copy link
Member

marcotc commented Jan 28, 2025

Looks great!

@p-datadog
Copy link
Member

Thank you!

@quinna-h quinna-h marked this pull request as ready for review January 29, 2025 15:19
@quinna-h quinna-h requested a review from a team as a code owner January 29, 2025 15:19
@p-datadog
Copy link
Member

Would be nice to see the generated markdown in this PR but if it's a lot of work I approve this PR as is and will review the markdown in a later PR.

@quinna-h
Copy link
Contributor Author

@p-datadog Thanks! I saw that you approved it, but the PR with the generated markdown table is here: #4236

bouwkast
bouwkast previously approved these changes Jan 29, 2025
Copy link
Contributor

@bouwkast bouwkast left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me

I think we'd want to follow up with product (specifically with httpx) and a follow up PR to add some tests for this.

.github/scripts/update_supported_versions.rb Show resolved Hide resolved
.github/scripts/update_supported_versions.rb Outdated Show resolved Hide resolved
.github/scripts/update_supported_versions.rb Outdated Show resolved Hide resolved
@bouwkast bouwkast dismissed their stale review February 3, 2025 18:10

trying to figure out merging issue

@marcotc marcotc merged commit 310b842 into master Feb 3, 2025
492 checks passed
@marcotc marcotc deleted the quinna.halim/update-supported-versions-script branch February 3, 2025 18:42
@github-actions github-actions bot added this to the 2.10.0 milestone Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/github Github repository maintenance and automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants