-
Notifications
You must be signed in to change notification settings - Fork 20
Improvement/arsn 492 #2371
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
base: development/8.2
Are you sure you want to change the base?
Improvement/arsn 492 #2371
Conversation
Hello benzekrimaha,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## development/8.2 #2371 +/- ##
===================================================
+ Coverage 69.91% 69.93% +0.01%
===================================================
Files 218 218
Lines 17440 17441 +1
Branches 3592 3593 +1
===================================================
+ Hits 12194 12198 +4
+ Misses 5242 5239 -3
Partials 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
@@ -3,7 +3,7 @@ | |||
"engines": { | |||
"node": ">=20" | |||
}, | |||
"version": "8.2.13", | |||
"version": "8.2.14", |
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.
not sure we want to release just yet, we got a few more that also need to reach 9.0 very soon.
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.
we will actually need to bump, for the s3utils PR
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.
yes we need to bump, but not on every PR. We currently have 3 PRs open in arsenal, all of which need to be bump'ed : so better to merge as many PRs as possible, and release/bump just once, after the last one of the batch.
18f1c2f
to
9f2d97f
Compare
67761a2
to
9926177
Compare
fcea66d
to
22a5462
Compare
@@ -411,7 +411,8 @@ class MongoClientInterface { | |||
value: { | |||
...newBucketMD, | |||
quotaMax: new Long(newBucketMD.quotaMax || '0'), | |||
capabilities: undefined, | |||
// We're passing the capabilities value here as it's used for testing purposes | |||
capabilities: newBucketMD.capabilities as CapabilitiesMongoDB || undefined, |
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.
that is very weird:
newBucketMD
is the "serializable" variant ofbucketMD
: if typing is needed, it should be done inBucketInfo.makeSerializable
- adding the type in here will only affect this specific function, as
payload
is only used here - Passing
bucketMD.capabilities
to createBucket() is not just for testing: today, maybe we do not do it "in prod" and only do it in tests, but since it is part of bucketInfo it should be supported, like all other fields... → if a fix is needed, it is not just for tests...
Issue: ARSN-492