-
Notifications
You must be signed in to change notification settings - Fork 1
Fix xmldom iteration and circular dependency in adt-client #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
550a1d1
fix: remove @abapify/adk from adt-client deps (circular dep, re-added…
Copilot 083897d
fix: checkout PR head in CI to avoid 3-way merge re-introducing circu…
Copilot 96fbec3
fix: exclude workspace root tsdown.config.ts from nx-tsdown plugin
claude 1d4a5dd
fix(ts-xsd): fix NamedNodeMap/NodeList iteration and resolve bugs
claude 6af06be
chore: add bun.lock generated by bun install
claude File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| # Nx CI Skill | ||
|
|
||
| Before committing, simulate what CI would do by running the same affected tasks locally. | ||
| This catches failures that would only surface after pushing. | ||
|
|
||
| ## How CI works | ||
|
|
||
| The main CI workflow (`.github/workflows/ci.yml`) runs these steps on every push/PR: | ||
|
|
||
| 1. `npx nx format:check` — checks formatting for all files | ||
| 2. `npx nx affected -t lint test build e2e-ci --parallel=3` — runs lint, test, build, e2e-ci | ||
| only for projects affected by the change (using `NX_BASE`/`NX_HEAD` set by `nrwl/nx-set-shas`) | ||
|
|
||
| ## Pre-commit: replicate CI locally with `nx affected` | ||
|
|
||
| Run these commands **before committing**, using the same targets CI uses: | ||
|
|
||
| ```bash | ||
| # 1. Check formatting (same as CI) | ||
| npx nx format:check | ||
|
|
||
| # 2. Find what is affected by your changes vs the base branch | ||
| # (omit --base if origin/main is already the default — check nx.json "defaultBase") | ||
| npx nx affected -t lint test build e2e-ci --parallel=3 --base=origin/main --head=HEAD | ||
| ``` | ||
|
|
||
| > **Why `affected` and not `run-many`?** | ||
| > `nx affected` only rebuilds and retests the projects touched by your diff, | ||
| > matching exactly what CI will do. `run-many` runs everything regardless of | ||
| > whether it changed, which is much slower and doesn't match CI behaviour. | ||
|
|
||
| > **`e2e-ci` locally**: The `e2e-ci` target is included to match CI faithfully. | ||
| > If no project in your workspace defines this target, Nx skips it silently. | ||
| > Remove it from the command if you want a faster pre-commit check that skips e2e. | ||
|
|
||
| ## Determine what is affected before running | ||
|
|
||
| To preview which projects and tasks will run without executing them: | ||
|
|
||
| ```bash | ||
| # Show affected projects | ||
| npx nx show projects --affected --base=origin/main --head=HEAD | ||
|
|
||
| # Dry-run to see the exact task list | ||
| npx nx affected -t lint test build e2e-ci --base=origin/main --head=HEAD --dry-run | ||
| ``` | ||
|
|
||
| > **Default base**: `nx affected` uses `defaultBase` from `nx.json` when `--base` is | ||
| > omitted. In this repo `nx.json` sets `"defaultBase": "main"`. On CI, `nrwl/nx-set-shas` | ||
| > overrides it via the `NX_BASE` / `NX_HEAD` environment variables. | ||
|
|
||
| ## Circular dependency check | ||
|
|
||
| If CI fails with "Could not execute command because the task graph has a circular | ||
| dependency", check for circular package dependencies: | ||
|
|
||
| ```bash | ||
| npx nx graph --file=/tmp/nx-graph.json | ||
| # then inspect /tmp/nx-graph.json for cycles | ||
| ``` | ||
|
|
||
| The most common cause is a package depending on another package that already | ||
| depends on it (e.g. `adt-client` ↔ `adk`). Fix by removing the erroneous | ||
| dependency from the package that should NOT depend on the other. | ||
|
|
||
| > **Why CI may still fail after the fix**: If the PR was branched from an older | ||
| > commit of `main` (before the bad dep was introduced), GitHub's auto-created | ||
| > merge commit (`refs/pull/N/merge`) re-adds the dep via 3-way merge semantics | ||
| > (base has no dep, main added it, your branch didn't touch it → merge keeps it). | ||
| > The CI workflow uses `ref: github.event.pull_request.head.sha` to check out | ||
| > the PR head instead of the merge commit, which avoids this problem. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Fork pr ci checkout
🐞 Bug⛯ ReliabilityAgent Prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools