Skip to content

Cleanup after prettier - #6305

Closed
francoisferrand wants to merge 9 commits into
improvement/CLDSRV-1002/prettier-whole-repofrom
improvement/CLDSRV-1002/cleanup-after-prettier
Closed

francoisferrand wants to merge 9 commits into
improvement/CLDSRV-1002/prettier-whole-repofrom
improvement/CLDSRV-1002/cleanup-after-prettier

Conversation

@francoisferrand

Copy link
Copy Markdown
Contributor

String cleanup after the Prettier reformat

The reformat to 120 columns left about 850 strings in the state they had been
hand-wrapped for the old 80-column limit: chains of short literals joined with
+ (often collapsed by Prettier onto one line, 'foo ' + 'bar'), messages
split as one short piece, several full pieces and another short one, and plain
strings concatenated with template literals. Prettier does not touch string
literals, so this has to be done by hand. These commits do it; string values
are preserved byte for byte, except for the last commit.

As a learning, we must also remember to run prettier/eslint in loop:

prettier --write  # bulk of the reformating
eslint --fix      # eslint to detect/fix issues, like useless concat
prettier --write  # again prettier, since eslint may have broken it...

Manual fixes

  • Merge string literals that were split for the old 80-column limit
  • Rewrap long strings to the 120-column limit
  • Fold plain strings into the adjacent template literals
  • Add the spaces missing between merged string pieces

Lint options to consider

  • no-useless-concat (ESLint core, currently not enabled): at the PR head it
    flagged 786 of the 849 concatenations found in review; the 63 it missed are
    multi-line chains, which the rule ignores by design. It also flags 294
    places nobody commented on, which would have to be fixed before enabling it.
  • prefer-template (enabled) ignores chains made only of literals, so
    'foo ' + ${bar}`` is never reported.
  • max-len is off in eslint.config.mjs. It is deprecated in ESLint core
    (still shipped in v9, removed in v10); @stylistic/max-len is the
    maintained replacement. With code: 120 and ignoreStrings: false it
    would report the 99 lines above; with ignoreStrings: true it reports
    nothing useful here.
  • Nothing detects an unbalanced wrap (short piece, full pieces, short piece);
    that needs a custom rule.

Issue: CLDSRV-1002

DarkIsDude and others added 9 commits September 21, 2026 14:43
Prettier now owns line length, so drop the eslint max-len rule it
conflicts with and the disable directives that went with it.

Issue: CLDSRV-1002
Generated with `yarn prettier:write`, no manual edit.

Issue: CLDSRV-1002
Prettier pads markdown table cells for alignment, which pushed this table
to 82 columns and tripped MD013. mdlint's config lives in the shared
Guidelines package and cannot be relaxed per repo, so shorten the widest
cell instead.

Issue: CLDSRV-1002
websiteHead.js and websiteHeadWithACL.js assert the ETag of index.html,
so reformatting these fixtures changes their MD5 and breaks the tests.
Their bytes are the test data, not source to style.

Issue: CLDSRV-1002
GitHub reads .git-blame-ignore-revs automatically; locally it needs
git config blame.ignoreRevsFile .git-blame-ignore-revs

Issue: CLDSRV-1002
Prettier reflowed the code to 120 columns, which left many messages and
test descriptions as chains of short literals joined with `+`. Merge
adjacent literals into a single string wherever the result fits; strings
that cannot fit are re-split into as few pieces as possible.

Issue: CLDSRV-1002
Strings that were wrapped by hand for the old 80-column limit kept their
short pieces after the Prettier reflow, often as one short line followed
by several full ones. Re-split them into as few balanced pieces as fit in
120 columns, or into a single literal when it fits.

Issue: CLDSRV-1002
Where a message was built as a plain string concatenated with a template
literal, merge the pieces into a single template literal. Kept separate
from the plain-string merges so it can be reviewed, or dropped, on its
own.

Issue: CLDSRV-1002
Merging the hand-wrapped literals exposed pieces that had been joined
without a separating space (and one doubled space), in test names and in
a few error messages. Add the missing spaces.

Issue: CLDSRV-1002
@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.66545% with 310 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.20%. Comparing base (7904b09) to head (37eb4eb).
⚠️ Report is 306 commits behind head on improvement/CLDSRV-1002/prettier-whole-repo.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
lib/api/objectGet.js 83.59% 21 Missing ⚠️
lib/api/objectCopy.js 89.88% 17 Missing ⚠️
lib/api/objectPutPart.js 85.08% 17 Missing ⚠️
lib/api/apiUtils/object/createAndStoreObject.js 77.46% 16 Missing ⚠️
lib/api/objectPutCopyPart.js 86.77% 16 Missing ⚠️
lib/api/apiUtils/object/objectLockHelpers.js 34.78% 15 Missing ⚠️
lib/api/completeMultipartUpload.js 90.13% 15 Missing ⚠️
lib/api/apiUtils/authorization/tagConditionKeys.js 63.33% 11 Missing ⚠️
lib/api/initiateMultipartUpload.js 84.37% 10 Missing ⚠️
lib/api/metadataSearch.js 47.36% 10 Missing ⚠️
... and 52 more
Additional details and impacted files

Impacted file tree graph

Files with missing lines Coverage Δ
lib/Config.js 80.35% <ø> (+0.17%) ⬆️
lib/api/apiUtils/authorization/serviceUser.js 100.00% <ø> (ø)
lib/api/apiUtils/bucket/bucketShield.js 100.00% <100.00%> (ø)
lib/api/apiUtils/bucket/createKeyForUserBucket.js 100.00% <ø> (ø)
...api/apiUtils/bucket/getReplicationConfiguration.js 92.30% <100.00%> (ø)
...b/api/apiUtils/bucket/validateReplicationConfig.js 84.61% <100.00%> (-1.10%) ⬇️
lib/api/apiUtils/bucket/validateSearch.js 96.42% <100.00%> (ø)
lib/api/apiUtils/integrity/validateChecksums.js 100.00% <ø> (ø)
lib/api/apiUtils/object/applyZenkoUserMD.js 80.00% <100.00%> (ø)
lib/api/apiUtils/object/checkHttpHeadersSize.js 88.88% <100.00%> (ø)
... and 152 more
@@                               Coverage Diff                               @@
##           improvement/CLDSRV-1002/prettier-whole-repo    #6305      +/-   ##
===============================================================================
- Coverage                                        85.32%   85.20%   -0.12%     
===============================================================================
  Files                                              206      206              
  Lines                                            13435    13412      -23     
===============================================================================
- Hits                                             11463    11428      -35     
- Misses                                            1972     1984      +12     
Flag Coverage Δ
file-ft-tests 68.30% <77.29%> (-0.07%) ⬇️
file-ft-tests-null-compat 68.82% <77.76%> (-0.10%) ⬇️
kmip-ft-tests 28.38% <11.84%> (+0.02%) ⬆️
mongo-v0-ft-tests 69.51% <78.13%> (-0.12%) ⬇️
mongo-v1-ft-tests 69.51% <78.13%> (-0.11%) ⬇️
multiple-backend 36.84% <17.73%> (+0.02%) ⬆️
s3c-ft-tests-v0 63.97% <74.77%> (-0.07%) ⬇️
s3c-ft-tests-v0-null-compat 64.03% <74.84%> (-0.04%) ⬇️
s3c-ft-tests-v1 63.95% <74.62%> (-0.04%) ⬇️
sur-tests 35.99% <17.76%> (-0.81%) ⬇️
sur-tests-inflights 37.83% <24.09%> (-0.01%) ⬇️
unit 71.22% <72.72%> (-0.05%) ⬇️
utapi-v2-tests 34.64% <23.98%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DarkIsDude
DarkIsDude force-pushed the improvement/CLDSRV-1002/prettier-whole-repo branch 2 times, most recently from 78bbc7c to a84bddd Compare September 24, 2026 09:08
@DarkIsDude DarkIsDude closed this Sep 24, 2026
@DarkIsDude

Copy link
Copy Markdown
Contributor

merged in #6297 (comment)

@DarkIsDude
DarkIsDude deleted the improvement/CLDSRV-1002/cleanup-after-prettier branch September 24, 2026 09:53
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