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

Upgrade steep and make it a bit more strict #4221

Merged
merged 4 commits into from
Dec 13, 2024
Merged

Upgrade steep and make it a bit more strict #4221

merged 4 commits into from
Dec 13, 2024

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Dec 11, 2024

Upgrade steep to 1.9.1 and rbs to 3.7.0, as I've encountered recent steep bugs that have been fixed in newer versions.

I also played a bit with the default type checking level: we currently use default but I've tried strict and noticed there are a few very helpful checks.
Most notably:

  • DifferentMethodParameterKind which checks that your .rbs declarations actually match the method parameters.
  • IncompatibleAssignment which checks if we are assigning incompatible types to fields we've explicitly typed already.

There's one functional change which steep caught in this upgrade: there was one dead code branch in lib/datadog/appsec/processor/rule_loader.rb, which I removed. This is actually dead code, since ip_passlist comes from a non-nillable configuration option.

Change log entry
None

@marcotc marcotc added the dev/internal Other internal work that does not need to be included in the changelog label Dec 11, 2024
@marcotc marcotc requested review from a team as code owners December 11, 2024 22:30
@github-actions github-actions bot added the appsec Application Security monitoring product label Dec 11, 2024
@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Dec 11, 2024

Datadog Report

Branch report: bump-steep
Commit report: 611bcf2
Test service: dd-trace-rb

✅ 0 Failed, 22094 Passed, 1457 Skipped, 5m 16.97s Total Time

@codecov-commenter
Copy link

codecov-commenter commented Dec 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.75%. Comparing base (65fb81d) to head (611bcf2).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4221   +/-   ##
=======================================
  Coverage   97.75%   97.75%           
=======================================
  Files        1355     1355           
  Lines       82165    82163    -2     
  Branches     4207     4207           
=======================================
- Hits        80321    80320    -1     
+ Misses       1844     1843    -1     

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

Copy link
Contributor

@sarahchen6 sarahchen6 left a comment

Choose a reason for hiding this comment

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

👍

Steepfile Show resolved Hide resolved
ruby-3.0.gemfile Outdated Show resolved Hide resolved
Copy link
Member

@Strech Strech left a comment

Choose a reason for hiding this comment

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

I think my suggestions are non-blocking, but gems versions are kind-a important

ruby-3.0.gemfile Outdated Show resolved Hide resolved
@pr-commenter
Copy link

pr-commenter bot commented Dec 13, 2024

Benchmarks

Benchmark execution time: 2024-12-13 19:08:38

Comparing candidate commit 611bcf2 in PR branch bump-steep with baseline commit 65fb81d in branch master.

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

@marcotc marcotc enabled auto-merge December 13, 2024 18:57
@marcotc marcotc disabled auto-merge December 13, 2024 18:57
@marcotc marcotc enabled auto-merge (squash) December 13, 2024 18:57
@marcotc marcotc merged commit ef81a2f into master Dec 13, 2024
349 checks passed
@marcotc marcotc deleted the bump-steep branch December 13, 2024 19:09
@github-actions github-actions bot added this to the 2.9.0 milestone Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appsec Application Security monitoring product dev/internal Other internal work that does not need to be included in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants