-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: upgrade urllib to 4.x #26
Conversation
WalkthroughThe pull request modifies the Continuous Integration (CI) workflow for a Node.js project by updating the Node.js version matrix in the Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected] |
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
test/OSSObject.test.ts (1)
1796-1804
: Consider preserving test cases as skipped testsRather than commenting out the test cases, consider using
it.skip()
to preserve them in the test suite while clearly indicating they are intentionally skipped. This helps maintain visibility of the test coverage gap.- // it.skip('should copy object from other bucket, sourceBucket is a separate parameter', async () => { - // const copySource = otherBucketObject; - // const copyTarget = `${prefix}oss-client/oss/has-bucket-name-copy-target.js`; - // const result = await ossObject.copy(copyTarget, copySource, otherBucket); - // assert.equal(result.res.status, 200); - // const info = await ossObject.head(copyTarget); - // assert.equal(info.status, 200); - // }); + it.skip('should copy object from other bucket, sourceBucket is a separate parameter', async () => { + const copySource = otherBucketObject; + const copyTarget = `${prefix}oss-client/oss/has-bucket-name-copy-target.js`; + const result = await ossObject.copy(copyTarget, copySource, otherBucket); + assert.equal(result.res.status, 200); + const info = await ossObject.head(copyTarget); + assert.equal(info.status, 200); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
test/OSSObject.test.ts
(2 hunks)
🔇 Additional comments (1)
test/OSSObject.test.ts (1)
1731-1732
: Clarify the reason for commenting out cross-bucket copy tests
The tests for copying objects between buckets have been commented out. Please clarify:
- Is this related to the urllib 4.x upgrade?
- Will this functionality be restored in a future update?
- Are there any compatibility issues with Node.js 18.19.0?
If these tests are temporarily disabled, consider:
- Adding TODO comments with tracking issues
- Documenting any API changes in the changelog
- Updating the documentation if this is a breaking change
Also applies to: 1786-1794
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #26 +/- ##
=======================================
Coverage 94.39% 94.39%
=======================================
Files 17 17
Lines 1891 1891
Branches 283 282 -1
=======================================
Hits 1785 1785
Misses 106 106 ☔ View full report in Codecov by Sentry. |
[skip ci] ## [2.4.0](v2.3.0...v2.4.0) (2024-12-05) ### Features * upgrade urllib to 4.x ([#26](#26)) ([ad9346f](ad9346f))
Summary by CodeRabbit
New Features
urllib
dependency to version 4.6.2.Chores
Tests
OSSObject
class, focusing on thecopy()
method and maintaining core functionality checks, including tests for non-English names and metadata handling.