Skip to content

Conversation

@mitjap
Copy link
Collaborator

@mitjap mitjap commented Feb 10, 2025

I didn't update documentation in #712.

@mitjap mitjap requested a review from Murderlon February 10, 2025 14:21
@changeset-bot
Copy link

changeset-bot bot commented Feb 10, 2025

🦋 Changeset detected

Latest commit: 280a6d4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@tus/s3-store Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

This comment was marked as resolved.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
packages/s3-store/README.md (1)

236-246: Improve sentence clarity in the Scaleway Object Storage example
The new example is very helpful for users configuring Scaleway Object Storage. However, the sentence on line 238 can be improved for grammatical clarity and flow. Consider rephrasing by adding a subject and a comma as suggested below to enhance readability.

-`@tus/s3-store` can be used with Scaleway Object Storage but with some additional configuration. Scaleway Object Storage has a limit of 1000 parts in a multipart upload.
+The `@tus/s3-store` package can be used with Scaleway Object Storage, but requires additional configuration. Specifically, Scaleway Object Storage imposes a limit of 1000 parts in a multipart upload.
🧰 Tools
🪛 LanguageTool

[style] ~238-~238: To form a complete sentence, be sure to include a subject.
Context: ...caleway Object Storage @tus/s3-store can be used with Scaleway Object Storage bu...

(MISSING_IT_THERE)


[uncategorized] ~238-~238: Possible missing comma found.
Context: ...can be used with Scaleway Object Storage but with some additional configuration. Sca...

(AI_HYDRA_LEO_MISSING_COMMA)

📜 Review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af6a55a and 341ce52.

📒 Files selected for processing (1)
  • packages/s3-store/README.md (2 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/s3-store/README.md

[style] ~238-~238: To form a complete sentence, be sure to include a subject.
Context: ...caleway Object Storage @tus/s3-store can be used with Scaleway Object Storage bu...

(MISSING_IT_THERE)


[uncategorized] ~238-~238: Possible missing comma found.
Context: ...can be used with Scaleway Object Storage but with some additional configuration. Sca...

(AI_HYDRA_LEO_MISSING_COMMA)

🔇 Additional comments (1)
packages/s3-store/README.md (1)

77-84: Documentation clarity for part size limits
The updated text now explains that the server may increase the part size to not exceed the options.maxMultipartParts limit, which improves configurability and clarity for users.

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

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

Thanks 👍

Unfortunately I don't use something like remark-toc to autogenerate the toc so you have to manually add this at the top.

[Example: use with Scaleway Object Storage](#example-use-with-scaleway-object-storage)

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
packages/s3-store/README.md (1)

237-244: New Scaleway Object Storage Example Added with Minor Language Suggestion

The example demonstrates the proper configuration for Scaleway Object Storage by setting maxMultipartParts to 1000, which aligns with the provider’s limitations. However, the descriptive sentence can be refined for improved clarity and grammatical completeness. For example:

-`@tus/s3-store` can be used with Scaleway Object Storage but with some additional configuration. Scaleway Object Storage has a limit of 1000 parts in a multipart upload.
+`@tus/s3-store` can be used with Scaleway Object Storage, but it requires additional configuration since Scaleway Object Storage has a limit of 1000 parts per multipart upload.

This minor improvement will address the LanguageTool feedback on sentence structure and punctuation.

🧰 Tools
🪛 LanguageTool

[style] ~239-~239: To form a complete sentence, be sure to include a subject.
Context: ...caleway Object Storage @tus/s3-store can be used with Scaleway Object Storage bu...

(MISSING_IT_THERE)


[uncategorized] ~239-~239: Possible missing comma found.
Context: ...can be used with Scaleway Object Storage but with some additional configuration. Sca...

(AI_HYDRA_LEO_MISSING_COMMA)

📜 Review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 185f63c and 6dc5290.

📒 Files selected for processing (1)
  • packages/s3-store/README.md (3 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/s3-store/README.md

[style] ~239-~239: To form a complete sentence, be sure to include a subject.
Context: ...caleway Object Storage @tus/s3-store can be used with Scaleway Object Storage bu...

(MISSING_IT_THERE)


[uncategorized] ~239-~239: Possible missing comma found.
Context: ...can be used with Scaleway Object Storage but with some additional configuration. Sca...

(AI_HYDRA_LEO_MISSING_COMMA)

🔇 Additional comments (3)
packages/s3-store/README.md (3)

17-17: New Scaleway Example Link in TOC Added

The new table of contents entry for the Scaleway Object Storage example is a welcome addition that helps users quickly locate the updated content. The anchor appears consistent with the header below.


78-79: Updated Reference in options.minPartSize Description

The text now dynamically refers to options.maxMultipartParts rather than a hardcoded S3 10K parts limit. This change improves clarity by linking the minimum part size behavior to the newly configurable maximum.


80-84: New options.maxMultipartParts Documentation Section

The added section clearly explains the new parameter. It specifies the default value and highlights the differences in limits among S3 providers. This addition enhances the documentation’s accuracy and usability.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
packages/s3-store/README.md (1)

237-247: Scaleway Object Storage Example – Language and Clarity Improvements
The Scaleway Object Storage example is a valuable addition. However, the explanatory text could be clarified for better readability. For example, instead of:

@tus/s3-store can be used with Scaleway Object Storage but with some additional configuration. Scaleway Object Storage has a limit of 1,000 parts in a multipart upload.

Consider rephrasing it to explicitly state the requirement. A suggested diff is provided below:

-`@tus/s3-store` can be used with Scaleway Object Storage but with some additional configuration. Scaleway Object Storage has a limit of 1,000 parts in a multipart upload.
+`@tus/s3-store` can be used with Scaleway Object Storage; however, additional configuration is required because Scaleway Object Storage limits multipart uploads to 1,000 parts.

This change addresses the static analysis hints regarding sentence structure and punctuation, making the instructions clearer and easier to follow.

🧰 Tools
🪛 LanguageTool

[style] ~239-~239: To form a complete sentence, be sure to include a subject.
Context: ...caleway Object Storage @tus/s3-store can be used with Scaleway Object Storage bu...

(MISSING_IT_THERE)


[uncategorized] ~239-~239: Possible missing comma found.
Context: ...can be used with Scaleway Object Storage but with some additional configuration. Sca...

(AI_HYDRA_LEO_MISSING_COMMA)

📜 Review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6dc5290 and 280a6d4.

📒 Files selected for processing (2)
  • .changeset/thin-keys-whisper.md (1 hunks)
  • packages/s3-store/README.md (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .changeset/thin-keys-whisper.md
🧰 Additional context used
🪛 LanguageTool
packages/s3-store/README.md

[style] ~239-~239: To form a complete sentence, be sure to include a subject.
Context: ...caleway Object Storage @tus/s3-store can be used with Scaleway Object Storage bu...

(MISSING_IT_THERE)


[uncategorized] ~239-~239: Possible missing comma found.
Context: ...can be used with Scaleway Object Storage but with some additional configuration. Sca...

(AI_HYDRA_LEO_MISSING_COMMA)

🔇 Additional comments (3)
packages/s3-store/README.md (3)

17-17: Added Scaleway Example Entry in Contents
The new bullet point for "Example: use with Scaleway Object Storage" has been correctly added to the Contents section. This improves discoverability of the new example.


78-79: Update to options.minPartSize Description
The description now properly references the new options.maxMultipartParts limit rather than the previously hardcoded "S3 10K parts limit." This update ensures that users understand the interplay between minPartSize and the newly introduced maxMultipartParts option.


80-85: Documentation of the New options.maxMultipartParts Option
The new section for options.maxMultipartParts is clear and concise. It explains the default value and why a custom limit might be needed for some S3-compatible providers. This addition aligns well with the recent code changes and helps users adjust configurations based on provider limitations.

@Murderlon Murderlon changed the title @tus/s3-store: update documentation @tus/s3-store: add missing docs for maxMultipartParts Feb 10, 2025
@Murderlon Murderlon merged commit 81eb03a into tus:main Feb 10, 2025
3 checks passed
@mitjap mitjap deleted the update-s3-store-documentation branch February 10, 2025 14:47
Murderlon added a commit that referenced this pull request Mar 24, 2025
* main:
  @tus/azure-store: fix non-ASCII characters error on metadata (#725)
  Fix package-lock.json
  @tus/s3-store: add missing docs for `maxMultipartParts` (#720)
  [ci] release (#719)
  @tus/server: don't use AbortSignal.any to fix memory leak (#718)
  [ci] release (#713)
  Add .coderabbit.yml
  @tus/s3-store: add `maxMultipartParts` option (#712)
  [ci] release (#701)
  @tus/s3-store: add `minPartSize` option (#703)
Murderlon added a commit that referenced this pull request Mar 25, 2025
* main:
  Refactor server to run in all (Node.js compatible) runtimes and meta frameworks (#710)
  [ci] release (#721)
  @tus/s3-store: fix unhandled promise rejection (#728)
  Bump @aws-sdk/client-s3 from 3.717.0 to 3.758.0 (#733)
  Bump @types/node from 22.10.1 to 22.13.7 (#734)
  Bump typescript from 5.6.2 to 5.8.2 (#731)
  @tus/azure-store: fix non-ASCII characters error on metadata (#725)
  Fix package-lock.json
  @tus/s3-store: add missing docs for `maxMultipartParts` (#720)
  [ci] release (#719)
  @tus/server: don't use AbortSignal.any to fix memory leak (#718)
  [ci] release (#713)
  Add .coderabbit.yml
  @tus/s3-store: add `maxMultipartParts` option (#712)
  [ci] release (#701)
  @tus/s3-store: add `minPartSize` option (#703)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants