Skip to content

Commit 63ed0bf

Browse files
authored
chore: reformat codebase with Prettier and add CI enforcement (#782)
Prettier has been installed as a devDependency but was never enforced — no format script, no CI check. This caused 30+ files to drift from the configured style, meaning PRs would randomly include formatting changes mixed with logic changes. ## Changes - **`package.json`**: Add `format` (`prettier --write .`) and `format:check` (`prettier --check .`) scripts - **`.prettierignore`**: Exclude `pnpm-lock.yaml`, `temp_*/`, and `docs/` from formatting - **`.github/workflows/lint.yml`**: Add a Format Check step before Type Check and Lint so PRs with formatting drift fail CI - **40 files reformatted**: Formatting-only changes (no logic modifications) across source, test, config, and markdown files
1 parent 1d8a5f5 commit 63ed0bf

40 files changed

Lines changed: 408 additions & 323 deletions

.github/workflows/build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ jobs:
1818
contents: read
1919
strategy:
2020
matrix:
21-
node: ['20','22']
21+
node: ['20', '22']
2222
name: Node ${{ matrix.node }}
2323
steps:
2424
- uses: actions/checkout@v6

.github/workflows/lint.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ jobs:
3333
- name: Install Dependencies
3434
if: steps.cache.outputs.cache-hit != 'true'
3535
run: pnpm install --frozen-lockfile
36+
- name: Format Check
37+
run: pnpm format:check
3638
- name: Type Check
3739
run: pnpm typecheck
3840
- name: Lint

.prettierignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
11
coverage/
22
dist/
33
node_modules/
4+
pnpm-lock.yaml
5+
temp_*/
6+
docs/

AGENTS.md

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,24 +113,26 @@ Some operations need explicit `isDryRun()` checks:
113113
- User experience optimizations (e.g., skipping sleep timers)
114114

115115
<!-- This section is maintained by the coding agent via lore (https://github.com/BYK/opencode-lore) -->
116+
116117
## Long-term Knowledge
117118

118-
### Gotcha
119+
### Architecture
120+
121+
<!-- lore:019cb31a-14ce-7892-b22a-0327cfcebc13 -->
119122

120-
<!-- lore:019ca05d-1ee2-759e-8a57-596470be9a3a -->
121-
* **Craft changelog: commits without PRs were forced into leftovers regardless of category match**: In \`src/utils/changelog.ts\`, the leftovers guard at the categorization step had \`if (!categoryTitle || !raw.pr)\` — the \`|| !raw.pr\` condition forced ALL commits without associated PRs into the "Other" leftovers section, even when they matched a category via \`commit\_patterns\` or labels. This meant direct pushes (no PR) like \`feat(auth): add SSO\` would correctly contribute to version bump calculation (which runs before the leftovers check) but would appear under "Other" in the changelog instead of their matched category. Fix: remove \`|| !raw.pr\` so only \`!categoryTitle\` controls leftover placement. The downstream rendering already handles PR-less commits gracefully — \`createPREntriesFromRaw\` uses \`raw.pr ?? ''\`, and \`formatChangelogEntry\` falls back to commit-hash links when \`prNumber\` is falsy (empty string).
123+
- **Registry target: repo_url auto-derived from git remote, not user-configurable**: \`repo_url\` in registry manifests is always set by Craft as \`https://github.com/${owner}/${repo}\`. Resolution: (1) explicit \`github: { owner, repo }\` in \`.craft.yml\` (rare), (2) fallback: auto-detect from git \`origin\` remote URL via \`git-url-parse\` library (\`git.ts:194-217\`, \`config.ts:286-316\`). Works with HTTPS and SSH remote URLs. Always overwritten on every publish — existing manifest values are replaced (\`registry.ts:417-418\`). Result is cached globally with \`Object.freeze\`. If remote isn't \`github.com\` and no explicit config exists, throws \`ConfigurationError\`. Most repos need no configuration — the git origin remote is sufficient.
122124

123-
<!-- lore:019c9ba5-515b-70c8-bc1a-1221f6858b42 -->
124-
* **Edit tool triggers Prettier reformatting on the entire file**: When using the Edit tool to make a targeted change to a file in a repo with Prettier configured, the tool may reformat the entire file (e.g., aligning markdown table columns, changing quote styles). This causes the git diff to show cosmetic changes far beyond the intended edit. To keep commits clean: either accept the Prettier reformatting as a net improvement (if the file passes \`prettier --check\`), or use \`git checkout -- \<file>\` to restore the original and re-apply the change via bash/sed for surgical precision. In this codebase, Prettier is configured (\`.prettierrc.yml\`) but the lint CI workflow does NOT run \`prettier --check\`, only ESLint and typecheck.
125+
<!-- lore:019cb31a-14c8-7ba9-b1c4-81b2e8bf7e85 -->
126+
127+
- **Registry target: urlTemplate generates artifact download URLs in manifest**: \`urlTemplate\` in the registry target config generates download URLs for release artifacts in the registry manifest's \`files\` field. Uses Mustache rendering with variables \`{{version}}\`, \`{{file}}\`, \`{{revision}}\`. Primarily useful for apps (standalone binaries) and CDN-hosted assets — SDK packages published to public registries (npm, PyPI, gem) typically don't need it. If neither \`urlTemplate\` nor \`checksums\` is configured, Craft skips adding file data entirely (warns at \`registry.ts:341-349\`). Real-world pattern: \`https://downloads.sentry-cdn.com/\<product>/{{version}}/{{file}}\`.
128+
129+
### Gotcha
125130

126-
<!-- lore:019c9eb7-a640-776a-929d-89120f81733a -->
127-
* **pnpm-lock.yaml merge conflicts: regenerate don't manually merge**: pnpm gotchas: (1) Lock file conflicts: never manually resolve — \`git checkout --theirs pnpm-lock.yaml\` then \`pnpm install\` to regenerate. \`git stash pop\` after merge can re-conflict; drop the stash and re-run instead. (2) Overrides: \`>=\` crosses major versions — use \`^\` to stay in-major. Version-range selectors don't reliably force re-resolution of compatible transitive deps; use blanket overrides when all consumers are on same major. (3) Overrides go stale on tree changes — audit with \`pnpm why\` and remove orphans.
131+
<!-- lore:019c9f57-aa0c-7a2a-8a10-911b13b48fc0 -->
128132

129-
### Pattern
133+
- **ESM modules prevent vi.spyOn of child_process.spawnSync — use test subclass pattern**: In ESM (Vitest or Bun), you cannot \`vi.spyOn\` exports from Node built-in modules — throws 'Module namespace is not configurable'. Workaround: create a test subclass that overrides the method calling the built-in and injects controllable values. \`vi.mock\` at module level works but affects all tests in the file.
130134

131-
<!-- lore:019c9ba5-515c-7131-a1e6-10f93443d0e2 -->
132-
* **AGENTS.md lore section: only include project-relevant entries**: The \`\<!-- lore-managed section -->\` in AGENTS.md is auto-maintained by the lore tool and can accumulate cross-project entries (React, Kubernetes, etc.) that are irrelevant to the Craft codebase. When committing AGENTS.md changes, review the lore section and strip entries that don't pertain to this repo. Only project-specific patterns (like the \`publish\_repo: self\` sentinel) should be included.
135+
<!-- lore:019c9be1-33d1-7b6e-b107-ae7ad42a4ea4 -->
133136

134-
<!-- lore:019c9b36-8f9d-71c9-a43a-d2715aa249d0 -->
135-
* **Craft publish\_repo 'self' sentinel resolves to GITHUB\_REPOSITORY at runtime**: The composite action's \`publish\_repo\` input supports a special sentinel value \`"self"\` which resolves to \`$GITHUB\_REPOSITORY\` at runtime in the bash script of the 'Request publish' step. This allows repos to create publish request issues in themselves rather than in a separate \`{owner}/publish\` repo. The resolution happens in bash (not in the GitHub Actions expression) because the expression layer sets \`PUBLISH\_REPO\` via \`inputs.publish\_repo || format('{0}/publish', github.repository\_owner)\` — the string \`"self"\` passes through as-is and gets resolved to the actual repo name in the shell. Useful for personal/small repos where the default GITHUB\_TOKEN already has write access to the repo itself.
137+
- **pnpm overrides with >= can cross major versions — use ^ to constrain**: pnpm overrides gotchas: (1) \`>=\` crosses major versions — use \`^\` to constrain within same major. (2) Version-range selectors don't reliably force re-resolution of compatible transitive deps; use blanket overrides when safe. (3) Overrides become stale — audit with \`pnpm why \<pkg>\` after dependency changes. (4) Never manually resolve pnpm-lock.yaml conflicts — \`git checkout --theirs\` then \`pnpm install\` to regenerate deterministically.
136138
<!-- End lore-managed section -->

CHANGELOG.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -356,15 +356,15 @@
356356

357357
### Bug Fixes 🐛
358358

359-
- fix(changelog): Unscoped entries should be grouped under "other" by @BYK in [#659](https://github.com/getsentry/craft/pull/659)
359+
- fix(changelog): Unscoped entries should be grouped under "other" by @BYK in [#659](https://github.com/getsentry/craft/pull/659)
360360

361361
### Build / dependencies / internal 🔧
362362

363363
- ci: Update action-prepare-release to v1.6.5 by @BYK in [#654](https://github.com/getsentry/craft/pull/654)
364364

365365
### Other
366366

367-
- fix(docker): Support regional Artifact Registry endpoints in isGoogleCloudRegistry by @BYK in [#661](https://github.com/getsentry/craft/pull/661)
367+
- fix(docker): Support regional Artifact Registry endpoints in isGoogleCloudRegistry by @BYK in [#661](https://github.com/getsentry/craft/pull/661)
368368

369369
## 2.13.1
370370

@@ -779,7 +779,7 @@
779779

780780
### Various fixes & improvements
781781

782-
- ref: Pin cocoapods version (#496) by @brustolin
782+
- ref: Pin cocoapods version (#496) by @brustolin
783783

784784
## 1.6.0
785785

@@ -795,14 +795,14 @@
795795
- build(deps): bump semver from 6.3.0 to 6.3.1 (#470) by @dependabot
796796
- build(deps): bump @babel/traverse from 7.22.5 to 7.23.2 (#494) by @dependabot
797797
- ref: remove volta from CI (#493) by @asottile-sentry
798-
- fix: Handle `{major}.json` and `{minor}.json` symlinks when publishing older versions (#483) by @cleptric
798+
- fix: Handle `{major}.json` and `{minor}.json` symlinks when publishing older versions (#483) by @cleptric
799799
- Bump symbol collector 1.12.0 (#491) by @bruno-garcia
800800

801801
## 1.4.4
802802

803803
### Various fixes & improvements
804804

805-
- fix(brew): Replace version in artifact names with '__VERSION__' to access checksums from mustache (#488) by @romtsn
805+
- fix(brew): Replace version in artifact names with '**VERSION**' to access checksums from mustache (#488) by @romtsn
806806

807807
## 1.4.3
808808

blog-post-draft.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ And for teams that prefer calendar-based versioning, Craft now supports `calver`
2323
versioning:
2424
policy: calver
2525
calver:
26-
format: "%y.%-m" # e.g., 24.12 for December 2024
26+
format: '%y.%-m' # e.g., 24.12 for December 2024
2727
```
2828
2929
The best part? When using auto-versioning, you get a preview in your PR with a "Semver Impact of This PR" section so you know exactly what _your specific change_ will do to the version.
@@ -202,5 +202,7 @@ Drop me a message, file an issue, or just leave a comment. I'm actively working
202202
LFG 🚀
203203

204204
[^1]: It's not actually living there. That would be creepy. It just reads the messages. Still creepy? OK moving on.
205+
205206
[^2]: It doesn't. But it handles reverts correctly, which is almost the same thing in software.
207+
206208
[^3]: The Vitest migration alone touches ~30 test files. Don't ask me how I know this.

build.mjs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@ if (process.env.SENTRY_AUTH_TOKEN) {
1515
assets: ['dist/craft.js', 'dist/craft.js.map'],
1616
filesToDeleteAfterUpload: ['dist/**/*.map'],
1717
},
18-
})
18+
}),
1919
);
2020
} else {
21-
console.log('[build] SENTRY_AUTH_TOKEN not found, skipping source map upload');
21+
console.log(
22+
'[build] SENTRY_AUTH_TOKEN not found, skipping source map upload',
23+
);
2224
}
2325

2426
// Build to .js file first so Sentry plugin can properly handle source maps
@@ -33,7 +35,9 @@ await esbuild.build({
3335
'import.meta.url': 'import_meta_url',
3436
'process.env.NODE_ENV': JSON.stringify('production'),
3537
...(process.env.CRAFT_BUILD_SHA && {
36-
'process.env.CRAFT_BUILD_SHA': JSON.stringify(process.env.CRAFT_BUILD_SHA),
38+
'process.env.CRAFT_BUILD_SHA': JSON.stringify(
39+
process.env.CRAFT_BUILD_SHA,
40+
),
3741
}),
3842
},
3943
outfile: 'dist/craft.js',

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@
8282
"clean": "rm -rf dist coverage",
8383
"lint": "eslint --cache --cache-strategy content",
8484
"fix": "pnpm lint --fix",
85+
"format": "prettier --write .",
86+
"format:check": "prettier --check .",
8587
"test": "vitest run",
8688
"test:watch": "vitest",
8789
"typecheck": "tsc --noEmit",

src/__mocks__/@aws-sdk/client-lambda.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
2-
31
const PUBLISHED_LAYER_TEST = {
42
Version: 1,
53
LayerVersionArn: 'test:layer:version:arn',

src/artifact_providers/base.ts

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ export interface ParsedFilterOptions {
8686
* Returns parsed the given raw filters.
8787
*/
8888
export function parseFilterOptions(
89-
rawFilters: RawFilterOptions
89+
rawFilters: RawFilterOptions,
9090
): ParsedFilterOptions {
9191
const parsedFilters: ParsedFilterOptions = {};
9292
if (rawFilters.includeNames) {
@@ -193,7 +193,7 @@ export abstract class BaseArtifactProvider {
193193
*/
194194
public async downloadArtifact(
195195
artifact: RemoteArtifact,
196-
downloadDirectory?: string
196+
downloadDirectory?: string,
197197
): Promise<string> {
198198
let finalDownloadDirectory;
199199
if (downloadDirectory) {
@@ -210,14 +210,14 @@ export abstract class BaseArtifactProvider {
210210
return cached;
211211
}
212212
this.logger.debug(
213-
`Downloading \`${artifact.filename}\` to \`${finalDownloadDirectory}\``
213+
`Downloading \`${artifact.filename}\` to \`${finalDownloadDirectory}\``,
214214
);
215215
const promise = this.doDownloadArtifact(
216216
artifact,
217-
finalDownloadDirectory
217+
finalDownloadDirectory,
218218
).catch(err => {
219219
this.logger.error(
220-
`Unable to download ${artifact.filename} from artifact provider!`
220+
`Unable to download ${artifact.filename} from artifact provider!`,
221221
);
222222
throw err;
223223
});
@@ -237,7 +237,7 @@ export abstract class BaseArtifactProvider {
237237
*/
238238
protected abstract doDownloadArtifact(
239239
artifact: RemoteArtifact,
240-
downloadDirectory: string
240+
downloadDirectory: string,
241241
): Promise<string>;
242242

243243
/**
@@ -254,12 +254,12 @@ export abstract class BaseArtifactProvider {
254254
*/
255255
public async downloadArtifacts(
256256
artifacts: RemoteArtifact[],
257-
downloadDirectory?: string
257+
downloadDirectory?: string,
258258
): Promise<string[]> {
259259
return Promise.all(
260260
artifacts.map(async artifact =>
261-
this.downloadArtifact(artifact, downloadDirectory)
262-
)
261+
this.downloadArtifact(artifact, downloadDirectory),
262+
),
263263
);
264264
}
265265

@@ -271,7 +271,7 @@ export abstract class BaseArtifactProvider {
271271
* @returns List of artifacts associated with that commit
272272
*/
273273
public async listArtifactsForRevision(
274-
revision: string
274+
revision: string,
275275
): Promise<RemoteArtifact[]> {
276276
this.logger.debug(`Fetching artifact list for revision \`${revision}\`.`);
277277
// check the cache first
@@ -288,7 +288,7 @@ export abstract class BaseArtifactProvider {
288288
artifacts = await this.fileListCache[revision];
289289
} catch (err) {
290290
this.logger.error(
291-
`Unable to retrieve artifact list for revision ${revision}!`
291+
`Unable to retrieve artifact list for revision ${revision}!`,
292292
);
293293
throw err;
294294
}
@@ -314,7 +314,7 @@ export abstract class BaseArtifactProvider {
314314
* be found
315315
*/
316316
protected abstract doListArtifactsForRevision(
317-
revision: string
317+
revision: string,
318318
): Promise<RemoteArtifact[]>;
319319

320320
/**
@@ -330,7 +330,7 @@ export abstract class BaseArtifactProvider {
330330
public async getChecksum(
331331
artifact: RemoteArtifact,
332332
algorithm: HashAlgorithm,
333-
format: HashOutputFormat
333+
format: HashOutputFormat,
334334
): Promise<string> {
335335
const filePath = await this.downloadArtifact(artifact);
336336
const checksumKey = `${algorithm}__${format}`;
@@ -356,7 +356,7 @@ export abstract class BaseArtifactProvider {
356356
*/
357357
public async filterArtifactsForRevision(
358358
revision: string,
359-
filterOptions?: ParsedFilterOptions
359+
filterOptions?: ParsedFilterOptions,
360360
): Promise<RemoteArtifact[]> {
361361
let filteredArtifacts = await this.listArtifactsForRevision(revision);
362362
if (!filterOptions || filteredArtifacts.length === 0) {
@@ -365,12 +365,12 @@ export abstract class BaseArtifactProvider {
365365
const { includeNames, excludeNames } = filterOptions;
366366
if (includeNames) {
367367
filteredArtifacts = filteredArtifacts.filter(artifact =>
368-
includeNames.test(artifact.filename)
368+
includeNames.test(artifact.filename),
369369
);
370370
}
371371
if (excludeNames) {
372372
filteredArtifacts = filteredArtifacts.filter(
373-
artifact => !excludeNames.test(artifact.filename)
373+
artifact => !excludeNames.test(artifact.filename),
374374
);
375375
}
376376
return filteredArtifacts;

0 commit comments

Comments
 (0)