Skip to content

replace legacy dependency #23

Merged
jessehanley merged 6 commits into
bentonow:mainfrom
ziptied:main
Apr 2, 2026
Merged

replace legacy dependency #23
jessehanley merged 6 commits into
bentonow:mainfrom
ziptied:main

Conversation

@ziptied

@ziptied ziptied commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

chore: replace legacy dependency and update to build in node fetch

ziptied added 6 commits April 2, 2026 13:54
Replace mock.module calls for 'cross-fetch' with direct
globalThis.fetch assignments. This simplifies test setup and
improves compatibility with modern fetch implementations.

Rename resetMockFetchTracking to cleanupMockFetch for clarity
and update imports accordingly.
Improve mock fetch cleanup by resetting originalFetch to
undefined after restoration. Add guard to prevent
overwriting originalFetch on subsequent calls. Wrap manual
fetch mocks in try-finally blocks to ensure proper cleanup.
Simplify variable declarations in mock fetch setup.
Add automated changelog version updates to the release workflow.
The workflow now updates the CHANGELOG.md version string to
match the new package version during releases. This ensures the
changelog stays synchronized with package.json and reduces manual
version management errors. The git commit now includes both
package.json and CHANGELOG.md files.
refactor: replace cross-fetch mocks with globalThis.fetch
Remove manual-testing.md and manual-validation.js files as these
testing guides and scripts are no longer maintained or needed for
the current development workflow. These files contained outdated
curl commands and SDK smoke test examples that have been superseded
by automated testing infrastructure.
docs: remove manual testing documentation
@greptile-apps

greptile-apps Bot commented Apr 2, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

Safe to merge after fixing the sed range issue in the release workflow.

The core cross-fetch → native fetch migration is correct and well-tested. One P1 defect exists: the CHANGELOG sed command will corrupt historical entries on the second release.

.github/workflows/release.yml – the sed substitution needs a 0,/pattern/ range guard.

Comments Outside Diff (2)

  1. .github/workflows/release.yml, line 9 (link)

    P1 sed replaces all version headers in CHANGELOG

    Without a range restriction, sed applies s/…/…/ to every line in the file. Once the CHANGELOG has more than one ## X.Y.Z header, every historical version entry will be overwritten with $VERSION, corrupting the history.

    Use a 0,/pattern/ address to restrict replacement to the first match only:

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: .github/workflows/release.yml
    Line: 9
    
    Comment:
    **`sed` replaces all version headers in CHANGELOG**
    
    Without a range restriction, `sed` applies `s/…/…/` to every line in the file. Once the CHANGELOG has more than one `## X.Y.Z` header, every historical version entry will be overwritten with `$VERSION`, corrupting the history.
    
    Use a `0,/pattern/` address to restrict replacement to the first match only:
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. .gitignore, line 31-32 (link)

    P2 docs/ and scripts/ gitignored but tracked in this PR

    docs/manual-testing.md, docs/manual-validation.js, and scripts/run-automation-validation.sh are committed in this same PR, so they remain tracked by git. However, any future files added under these directories will be silently ignored, which may surprise contributors expecting their additions to show up in git status.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: .gitignore
    Line: 31-32
    
    Comment:
    **`docs/` and `scripts/` gitignored but tracked in this PR**
    
    `docs/manual-testing.md`, `docs/manual-validation.js`, and `scripts/run-automation-validation.sh` are committed in this same PR, so they remain tracked by git. However, any future files added under these directories will be silently ignored, which may surprise contributors expecting their additions to show up in `git status`.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/release.yml
Line: 9

Comment:
**`sed` replaces all version headers in CHANGELOG**

Without a range restriction, `sed` applies `s/…/…/` to every line in the file. Once the CHANGELOG has more than one `## X.Y.Z` header, every historical version entry will be overwritten with `$VERSION`, corrupting the history.

Use a `0,/pattern/` address to restrict replacement to the first match only:

```suggestion
        run: sed -i "0,/^## [0-9]\+\.[0-9]\+\.[0-9]\+/s/^## [0-9]\+\.[0-9]\+\.[0-9]\+/## $VERSION/" CHANGELOG.md
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .gitignore
Line: 31-32

Comment:
**`docs/` and `scripts/` gitignored but tracked in this PR**

`docs/manual-testing.md`, `docs/manual-validation.js`, and `scripts/run-automation-validation.sh` are committed in this same PR, so they remain tracked by git. However, any future files added under these directories will be silently ignored, which may surprise contributors expecting their additions to show up in `git status`.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Merge pull request #12 from ziptied/BEN-..." | Re-trigger Greptile

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c3c56db995

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

run: npm version $VERSION --no-git-tag-version

- name: Update CHANGELOG version
run: sed -i "s/^## [0-9]\+\.[0-9]\+\.[0-9]\+/## $VERSION/" CHANGELOG.md

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Limit changelog version bump to the latest entry

The sed command currently replaces every matching ## x.y.z heading in CHANGELOG.md, so once the changelog has more than one release section, running this workflow rewrites historical headings to the new $VERSION as well. This silently corrupts version history (e.g., multiple sections end up labeled with the same version) and can produce misleading changelog/release metadata; the substitution should target only the first matching heading.

Useful? React with 👍 / 👎.

@jessehanley jessehanley left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@jessehanley jessehanley merged commit a2d1042 into bentonow:main Apr 2, 2026
3 checks passed
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