fix: fix atom building script #413
Conversation
KazuCocoa
changed the title
| @@ -6,9 +6,9 @@ export function createRemoteDebugger<T extends boolean> ( | |||
| opts: T extends true ? RemoteDebuggerRealDeviceOptions : RemoteDebuggerOptions, | |||
| realDevice: T | |||
| ): T extends true ? RemoteDebuggerRealDevice : RemoteDebugger { | |||
| // @ts-ignore TS does not understand that | |||
There was a problem hiding this comment.
This line kept showing below here...:
> appium-remote-debugger@12.1.6 build
> tsc -b
index.ts:12:7 - error TS2322: Type 'RemoteDebugger' is not assignable to type 'T extends true ? RemoteDebuggerRealDevice : RemoteDebugger'.
12 : new RemoteDebugger(opts);
~~~~~~~~~~~~~~~~~~~~~~~~
scripts/common.mjs
Outdated
| const {stdout, stderr} = await exec('bazel', ['--version']); | ||
| if (stderr) { | ||
| throw new Error(`Please setup Bazel ${bazelVersion} runtime environment. ` + | ||
| `Current environment got ${stderr} with 'bazel --version' call.`); |
There was a problem hiding this comment.
I am not sure if the actual stderr would look readable as part of the error message. I usually put such things to the very end of a sentence after a colon, for example ...Original error: ${stderr}
scripts/common.mjs
Outdated
| `Current environment got ${stderr} with 'bazel --version' call.`); | ||
| } | ||
| // e.g. "bazel 7.4.1" | ||
| const currentBazelVersion = stdout.trim().split(' ')[1]; |
There was a problem hiding this comment.
not sure if it would make more sense to use semver for this purpose
There was a problem hiding this comment.
yea, I'm not sure Bazel keeps maintaining them with semiver, so currently I did complete match only
scripts/common.mjs
Outdated
| const bazelVersion = (await fs.readFile(BAZEL_VERSION, 'utf8')).trim(); | ||
| const {stdout, stderr} = await exec('bazel', ['--version']); | ||
| if (stderr) { | ||
| throw new Error(`Please setup Bazel ${bazelVersion} runtime environment. ` + |
There was a problem hiding this comment.
maybe we could also provide a help URL here on how to deploy bazel using, for example, brew
| await exec('bazel', ['build', target], {cwd: SELENIUM_DIRECTORY}); | ||
| } | ||
| log(`Bazel builds complete`); | ||
| log.info(`Bazel builds complete`); | ||
| } | ||
|
|
||
| async function atomsCopyAtoms (atomsDir, fileFilter = () => true) { |
There was a problem hiding this comment.
it might be useful to annotate these functions with jsdoc types
scripts/common.mjs
Outdated
| try { | ||
| result = await exec('bazel', ['--version']); | ||
| } catch (e) { | ||
| throw new Error(`Bazel version check with 'bazel --version' got an error: ${e}`); |
There was a problem hiding this comment.
perhaps it would be more useful to replace ${e} with ${e.stderr || e.message}
scripts/common.mjs
Outdated
| @@ -118,27 +119,28 @@ async function atomsCopyAtoms (atomsDir, fileFilter = () => true) { | |||
| // convert - to _ for backwards compatibility with old atoms | |||
| const newFileName = path.basename(file).replace('-ios', '').replace(/-/g, '_'); | |||
| const to = path.join(ATOMS_DIRECTORY, newFileName); | |||
| log(`Copying ${file} to ${to}`); | |||
| log.info(`Copying ${file} to ${to}`); | |||
| // delete an existing file if it was put here by an earlier run of the function, to enable | |||
| // overwriting | |||
| try { | |||
There was a problem hiding this comment.
maybe this whole try/catch could be replaced with rimraf
scripts/common.mjs
Outdated
| /** | ||
| * Copy atoms in bazel built result to 'atoms' in this repository's main 'atoms' place. | ||
| * @param {string} atomsDir | ||
| * @param {string} fileFilter |
There was a problem hiding this comment.
fileFilter is a function
## [12.1.7](v12.1.6...v12.1.7) (2025-03-04) ### Bug Fixes * fix atom building script ([#413](#413)) ([adb6f15](adb6f15))
|
🎉 This PR is included in version 12.1.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
mjsI assume the primary intention of SELENIUM_BRANCH and SELENIUM_GITHUB are to keep current latest built target in code, so probably it makes sense to keep them in code while allowing us to give an arbitrary pair of them via env var makes sense to build without code changes.