Skip to content

HDDS-16010. Fix lint silently failing and fix existing lint errors - #516

Merged
adoroszlai merged 14 commits into
apache:masterfrom
errose28:fix-lint
Jul 29, 2026
Merged

HDDS-16010. Fix lint silently failing and fix existing lint errors#516
adoroszlai merged 14 commits into
apache:masterfrom
errose28:fix-lint

Conversation

@errose28

@errose28 errose28 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Problem

Since HDDS-14546, CI has been passing even with lint errors because pnpm run lint always returns 0. It is defined as

  "lint": "eslint . ; markdownlint . ; yamllint --format=colored ."

Claude's explanation which is more thorough than mine:

The three tools are chained with ;, not &&. In a shell, a ;-separated sequence returns the exit code of the last command only. So:

eslint fails → non-zero code, but ignored; next command runs anyway.
markdownlint fails → non-zero code, but ignored; next command runs anyway.
The overall exit code of pnpm run lint = whatever yamllint returns.
So unless yamllint itself fails, the Run lint step exits 0 and CI stays green — even with eslint or markdownlint errors present. markdownlint is genuinely running (you'd see its errors in the log output), but its failure is swallowed.

Fixes

  1. Use an rc variable to track the aggregate return code of all commands in pnpm run lint. It starts as 0 and is set to 1 if any commands fail.

  2. Update markdownlint capitalization check to allow "s3" as lowercase when it is not used in sentences (preceded by a space). This matches the convention for other words there like "ozone".

  3. eslint has been silently failing since it was added in HDDS-14770. unrs-resolver actually requires a package it declares as optional (seems like an upstream mistake but we are stuck with it either way). Lint checks were installing dependencies with --dev to avoid installing build tools not required at that stage, but this also drops optional dependencies.

  [napi-postinstall@0.3.4] Failed to find package "@unrs/resolver-binding-linux-x64-gnu" on the file system                                                                                     
                                                                                                                                                                                                
  This can happen if you use the "--no-optional" flag. The "optionalDependencies"                                                                                                               
  package.json feature is used by unrs-resolver to install the correct napi binary                                                                                                              
  for your current platform. This install script will now attempt to work around                                                                                                                
  this. If that fails, you need to remove the "--no-optional" flag to use unrs-resolver.                                                                                                        
                                                                                                                                                                                                
  [napi-postinstall@0.3.4] Trying to install package "@unrs/resolver-binding-linux-x64-gnu" using npm                                                                                           
                                                                                                                                                                                                
  Oops! Something went wrong! :(                                                                                                                                                                
                                                                                                                                                                                                
  ESLint: 10.0.2                                                                                                                                                                                
                                                                                                                                                                                                
  Error: Cannot find native binding. npm has a bug related to optional dependencies (https://github.com/npm/cli/issues/4828). Please try `npm i` again after removing both package-lock.json    
  and node_modules directory.                                                                                                                                                                   
      at Object.<anonymous> (/home/runner/work/ozone-site/ozone-site/node_modules/.pnpm/unrs-resolver@1.11.1/node_modules/unrs-resolver/index.js:376:11)                                        
      at Module._compile (node:internal/modules/cjs/loader:1521:14)                                                                                                                             
      at Module._extensions..js (node:internal/modules/cjs/loader:1623:10)                                                                                                                      
      at Module.load (node:internal/modules/cjs/loader:1266:32)                                                                                                                                 
      at Module._load (node:internal/modules/cjs/loader:1091:12)                                                                                                                                
      at cjsLoader (node:internal/modules/esm/translators:298:15)                                                                                                                               
      at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:240:7)                                                                                                                   
      at ModuleJob.run (node:internal/modules/esm/module_job:325:25)                                                                                                                            
      at async ModuleLoader.import (node:internal/modules/esm/loader:606:24)                                                                                                                    
      at async dynamicImportConfig (/home/runner/work/ozone-site/ozone-site/node_modules/.pnpm/eslint@10.0.2_jiti@1.21.7/node_modules/eslint/lib/config/config-loader.js:185:17)                
   ELIFECYCLE  Command failed with exit code 1.  
  1. Added --frozen-lockfile to all static check install commands so that they use the pinned versions.

  2. All existing lint errors that previously slipped in have been fixed in this PR.

What is the link to the Apache Jira?

HDDS-16010

How was this patch tested?

  • Local run of pnpm run lint:
    • Before this change: exits 0 but prints errors
    • With the correct command only: exits 1 and prints errors
    • After this whole change with fixes: Exits 0 and prints no errors.

@errose28
errose28 requested a review from adoroszlai July 28, 2026 17:21
@errose28

Copy link
Copy Markdown
Contributor Author

cc @yuriipalam

@errose28
errose28 marked this pull request as ready for review July 28, 2026 17:22
jojochuang and others added 2 commits July 28, 2026 13:42
Address remaining MD044/MD029/MD028 issues in docs added on master after the initial lint sweep.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread docs/08-developer-guide/04-project/04-enhancement-proposal.md Outdated
jojochuang and others added 3 commits July 28, 2026 15:14
…-short-circuit-local-reads.md

Co-authored-by: Ethan Rose <33912936+errose28@users.noreply.github.com>
…03-merged-branches/16-hdds-8342-s3-lifecycle-configurations.md

Co-authored-by: Ethan Rose <33912936+errose28@users.noreply.github.com>
Keeps template steps in order when blockquotes and notes sit between sections.

Co-authored-by: Cursor <cursoragent@cursor.com>
@adoroszlai
adoroszlai merged commit 945eeb9 into apache:master Jul 29, 2026
13 checks passed
@adoroszlai

Copy link
Copy Markdown
Contributor

Thanks @errose28, @jojochuang for the patch.

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.

3 participants