Skip to content

Commit f0de9e8

Browse files
Generated best practices file
1 parent d349baa commit f0de9e8

1 file changed

Lines changed: 144 additions & 0 deletions

File tree

best_practices.md

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
2+
<b>Pattern 1: Prefer invoking external tools via a centralized helper that accepts a binary plus an args array (not shell strings), and wrap failures with a consistent, user-facing error message while attaching the original error via `{ cause: error }`.
3+
</b>
4+
5+
Example code before:
6+
```
7+
import { execSync } from "node:child_process";
8+
9+
function runPip(pipBin, depNames) {
10+
// shell string + loses original error context
11+
return execSync(`${pipBin} show ${depNames.join(" ")}`).toString();
12+
}
13+
```
14+
15+
Example code after:
16+
```
17+
import { invokeCommand } from "./tools.js";
18+
19+
function runPip(pipBin, depNames) {
20+
try {
21+
return invokeCommand(pipBin, ["show", ...depNames], { stdio: "pipe" }).toString("utf-8");
22+
} catch (error) {
23+
throw new Error("Failed to invoke 'pip show' to fetch package metadata.", { cause: error });
24+
}
25+
}
26+
```
27+
28+
<details><summary>Examples for relevant past discussions:</summary>
29+
30+
- https://github.com/guacsec/trustify-da-javascript-client/pull/185#discussion_r2058479181
31+
- https://github.com/guacsec/trustify-da-javascript-client/pull/159#discussion_r2029025976
32+
- https://github.com/guacsec/trustify-da-javascript-client/pull/206#discussion_r2100328541
33+
</details>
34+
35+
36+
___
37+
38+
<b>Pattern 2: Avoid global process state changes (like `process.chdir`) for cross-platform command execution; prefer passing `cwd`/`env` via subprocess options, and use OS-correct primitives (e.g., `path.delimiter`) when manipulating PATH.
39+
</b>
40+
41+
Example code before:
42+
```
43+
const originalDir = process.cwd();
44+
process.chdir(projectDir);
45+
try {
46+
invokeCommand("npm", ["install"]);
47+
} finally {
48+
process.chdir(originalDir);
49+
}
50+
51+
process.env.PATH = `${extraPaths.join(":")}:${process.env.PATH}`;
52+
```
53+
54+
Example code after:
55+
```
56+
invokeCommand("npm", ["install"], { cwd: projectDir });
57+
58+
const newPath = `${extraPaths.join(path.delimiter)}${path.delimiter}${process.env.PATH}`;
59+
invokeCommand("node", ["--version"], { env: { ...process.env, PATH: newPath } });
60+
```
61+
62+
<details><summary>Examples for relevant past discussions:</summary>
63+
64+
- https://github.com/guacsec/trustify-da-javascript-client/pull/182#discussion_r2057977443
65+
- https://github.com/guacsec/trustify-da-javascript-client/pull/191#discussion_r2068393651
66+
- https://github.com/guacsec/trustify-da-javascript-client/pull/206#discussion_r2100324334
67+
</details>
68+
69+
70+
___
71+
72+
<b>Pattern 3: Make CI/release automation deterministic and non-recursive: ensure workflows don’t trigger themselves (tag/branch loops), generate EA/dev versions at build time (e.g., `-ea.<short_sha>`), and keep workflow comments and conditions aligned with actual behavior.
73+
</b>
74+
75+
Example code before:
76+
```
77+
on:
78+
push:
79+
tags: ["*"] # too broad, may trigger unintended publishes
80+
81+
# For branch pushes, use -dev suffix
82+
VERSION="${BASE}-dev"
83+
git tag "$VERSION"
84+
git push origin "$VERSION" # can trigger same workflow again
85+
```
86+
87+
Example code after:
88+
```
89+
on:
90+
push:
91+
tags: ["v*.*.*"] # only release tags
92+
93+
# For main EA builds, use -ea.<short_sha> suffix
94+
BASE=$(node -p "require('./package.json').version" | sed -E 's/-ea[.-][0-9]+$//')
95+
SHORT_SHA=$(git rev-parse --short "${GITHUB_SHA}")
96+
VERSION="${BASE}-ea.${SHORT_SHA}"
97+
98+
# Avoid creating/pushing tags from workflows unless explicitly required
99+
```
100+
101+
<details><summary>Examples for relevant past discussions:</summary>
102+
103+
- https://github.com/guacsec/trustify-da-javascript-client/pull/274#discussion_r2533854698
104+
- https://github.com/guacsec/trustify-da-javascript-client/pull/277#discussion_r2534926978
105+
- https://github.com/guacsec/trustify-da-javascript-client/pull/276#discussion_r2534393333
106+
</details>
107+
108+
109+
___
110+
111+
<b>Pattern 4: Keep configuration and terminology explicit and externally configurable: avoid hardcoded endpoints/ambiguous docs, prefer clear environment variable naming, and only set/emit optional request parameters when they change behavior (e.g., send `recommend=false` only when disabled).
112+
</b>
113+
114+
Example code before:
115+
```
116+
// Hardcoded backend + ambiguous docs
117+
const BACKEND = "https://example.stage.internal";
118+
// "task param" (unclear) in README
119+
120+
// Always send recommend param even when default
121+
const finalUrl = `${url}/api/v4/analysis?recommend=true`;
122+
```
123+
124+
Example code after:
125+
```
126+
// Backend configured via env/opts with documented precedence
127+
const backend = getCustom("TRUSTIFY_DA_BACKEND_URL", "https://prod.example.com", opts);
128+
129+
// Only include query parameter when disabling recommendations
130+
const finalUrl = new URL(`${backend}/api/v4/analysis`);
131+
if (opts.TRUSTIFY_DA_RECOMMENDATIONS_ENABLED === "false") {
132+
finalUrl.searchParams.set("recommend", "false");
133+
}
134+
```
135+
136+
<details><summary>Examples for relevant past discussions:</summary>
137+
138+
- https://github.com/guacsec/trustify-da-javascript-client/pull/353#discussion_r2708314483
139+
- https://github.com/guacsec/trustify-da-javascript-client/pull/249#discussion_r2499762275
140+
- https://github.com/guacsec/trustify-da-javascript-client/pull/233#discussion_r2157547122
141+
</details>
142+
143+
144+
___

0 commit comments

Comments
 (0)