-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add Rails 7.1 #2242
Add Rails 7.1 #2242
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable to me. I don't know a lot about system-tests, but can confirm this is effectively a copy paste of the Rails 7.0 image, updated to 7.1 and with Ruby 3.2.
|
||
|
||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor:
``` | |
```bash |
Yeah given the timing I outright copied the app code from 7.0, while in a perfect world this should be a brand new app as generated by |
IIUC Rails 4.2 failure is solved by DataDog/dd-trace-rb#3541 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's failing on some ruby runs, but I don't know if it's related ?
The rails42 ones are because of DataDog/dd-trace-rb#3542 The rails40 prod should be unrelated as well, it just failed to start the container for some reason and there's no reason it wouldn't start yet still start for all other scenarios + for dev on the same scenario. |
ok, approving |
Motivation
missing coverage
Changes
add rails 7.1 on ruby 3.2 + rack 3 + puma 6
Workflow
codeowners
file quickly.🚀 Once your PR is reviewed, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
run-parametric-scenario
,run-profiling-scenario
...) are presentsbuild-XXX-image
label is present