Skip to content

docs: include ELECTRON_BUILD_TOOLS_ROOT in README - #905

Open
rf-figma wants to merge 1 commit into
mainfrom
rf-figma/document-root-var
Open

rf-figma wants to merge 1 commit into
mainfrom
rf-figma/document-root-var

Conversation

@rf-figma

Copy link
Copy Markdown
Member

This is used by the installer to decide where the whole EBT setup should live. It's currently not documented; since the installer's README just points to this repo, this seems like the right place for it.

I also fixed a small discrepancy Claude pointed out -- we had an incorrect default value documented for DEPOT_TOOLS_DIR.

This is used by the [installer] to decide where the whole EBT setup
should live. It's currently not documented; since the installer's README
just points to this repo, this seems like the right place for it.

I also fixed a small discrepancy Claude pointed out -- we had an
incorrect default value documented for `DEPOT_TOOLS_DIR`.

[installer]: https://github.com/electron/build-tools-installer
@rf-figma
rf-figma requested review from a team and ckerr as code owners September 15, 2026 15:53

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good — straightforward documentation fix, verified accurate against the source. Checked download-dist's default output dir against src/utils/download-dist.ts (resolves relative to the build-tools install dir, matching the new <build-tools>/artifacts/... wording) and DEPOT_TOOLS_DIR's default against src/utils/depot-tools.ts (path.resolve(..., 'third_party', 'depot_tools'), matching the corrected default). The new ELECTRON_BUILD_TOOLS_ROOT row isn't read anywhere in this repo's source, consistent with the PR description that it's consumed by an external installer.

Extended reasoning...

Overview

The diff touches only README.md, updating two documented defaults (download-dist -o/--output and DEPOT_TOOLS_DIR) and adding one new documented env var (ELECTRON_BUILD_TOOLS_ROOT). No source files changed.

Security risks

None — documentation-only change with no executable content.

Level of scrutiny

Low. This is a trivial, mechanical text change with no functional impact. I verified the factual accuracy of the doc updates directly against the source: src/utils/download-dist.ts resolves the artifacts directory relative to import.meta.dirname (i.e., the build-tools install directory), confirming the new <build-tools>/artifacts/... wording is correct (the old ~/.electron_build_tools/artifacts/... wording was stale, and in fact still appears uncorrected in some CLI help strings in e-download-dist.ts/e-pr.ts, but that pre-existing inconsistency is out of scope for this PR). src/utils/depot-tools.ts defines defaultDepotPath as path.resolve(..., 'third_party', 'depot_tools'), confirming the corrected DEPOT_TOOLS_DIR default.

Other factors

ELECTRON_BUILD_TOOLS_ROOT is not referenced anywhere in this repo's source, but per the PR description it is consumed by an external installer tool that decides where to install build-tools before this repo's own code runs, so its absence from this codebase is expected rather than a documentation error. No prior review comments exist on this PR to reconcile, and the bug hunter reported no findings.

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