Skip to content

Remove stale TS CLI References#539

Merged
MGudgin merged 1 commit into
mainfrom
user/gudge/remove_stale_ts_cli_refs
Jun 23, 2026
Merged

Remove stale TS CLI References#539
MGudgin merged 1 commit into
mainfrom
user/gudge/remove_stale_ts_cli_refs

Conversation

@MGudgin

@MGudgin MGudgin commented Jun 19, 2026

Copy link
Copy Markdown
Member

Remove references to the TypeScript CLI from copilot-instructions.md

Microsoft Reviewers: Open in CodeFlow

Copilot AI review requested due to automatic review settings June 19, 2026 15:46
@MGudgin MGudgin requested a review from a team as a code owner June 19, 2026 15:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Not ready to approve

The updated doc section still states TypeScript targets ES2020/CommonJS, which conflicts with the SDK’s current ES2022/NodeNext (ESM) configuration and should be corrected while touching this area.

Pull request overview

This PR updates the repository’s contributor instructions by removing stale references to a TypeScript CLI that no longer exists in this repo, keeping .github/copilot-instructions.md aligned with the current project structure (Rust core + TypeScript SDK).

Changes:

  • Removes CLI-specific build, lint, and test commands from the Copilot instructions.
  • Updates wording to describe the TypeScript layer as SDK-only (not SDK/CLI).
  • Tweaks the Linux --rust-only description to reflect skipping the SDK (not SDK/CLI).
File summaries
File Description
.github/copilot-instructions.md Removes stale TypeScript CLI references from build/lint/test guidance and updates architecture wording to SDK-only.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

Comment thread .github/copilot-instructions.md Outdated
@MGudgin MGudgin force-pushed the user/gudge/remove_stale_ts_cli_refs branch from 0c2726b to e4cdbd4 Compare June 19, 2026 16:10
@MGudgin

MGudgin commented Jun 19, 2026

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@microsoft microsoft deleted a comment from azure-pipelines Bot Jun 19, 2026
@MGudgin

MGudgin commented Jun 22, 2026

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@MGudgin MGudgin closed this Jun 22, 2026
@MGudgin MGudgin reopened this Jun 22, 2026
@MGudgin MGudgin force-pushed the user/gudge/remove_stale_ts_cli_refs branch from e4cdbd4 to e4eff33 Compare June 22, 2026 21:53
@bbonaby

bbonaby commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

I think for this one it's because we have .md files set to ignore in here:

paths-ignore:

But then in the Github actions in the settings for the repository, I have all the GitHub actions set to required. In the azure pipelines we have the same thing as well here:


That's why when you commented azp run the bot said Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

We don't require the azure pipelines to succeed before merging so that one is fine. Unfortunately the GitHub pipelines will stay stuck forever waiting on the build to run even though we have these paths set to ignore. E.g Github waiting for required pipelines to succeed but they never run because they're skipped. You can see the required status checks here: https://github.com/microsoft/mxc/rules/16396467 under Require status checks to pass.

We could remove the required tags from these Github pipelines in the settings to prevent this, as I believe the pipelines would show up as skipped in this case for .md only PRs. That said, we'd have to be very careful approving pull requests in the future since that would allow PR builds to fail but allow someone to still check it in regardless.

OR

We can add a bypass to the PR rule. Note: the PR rule is really just to make sure the pipelines are required to pass at PR time. There is a Microsoft rule that PRs require at least 1 approver which we also have and isn't bypassable here, so effectively bypassing would be just for the PR rule and we can scope it to only a few people with it documented that it should only be used for document changes.

I'm good with either two. Thoughts?

Note: This one also has the same issue: #545

@MGudgin

MGudgin commented Jun 23, 2026

Copy link
Copy Markdown
Member Author

I think for this one it's because we have .md files set to ignore in here:

paths-ignore:

But then in the Github actions in the settings for the repository, I have all the GitHub actions set to required. In the azure pipelines we have the same thing as well here:

That's why when you commented azp run the bot said Azure Pipelines could not run because the pipeline triggers exclude this branch/path.
We don't require the azure pipelines to succeed before merging so that one is fine. Unfortunately the GitHub pipelines will stay stuck forever waiting on the build to run even though we have these paths set to ignore. E.g Github waiting for required pipelines to succeed but they never run because they're skipped. You can see the required status checks here: https://github.com/microsoft/mxc/rules/16396467 under Require status checks to pass.

We could remove the required tags from these Github pipelines in the settings to prevent this, as I believe the pipelines would show up as skipped in this case for .md only PRs. That said, we'd have to be very careful approving pull requests in the future since that would allow PR builds to fail but allow someone to still check it in regardless.

OR

We can add a bypass to the PR rule. Note: the PR rule is really just to make sure the pipelines are required to pass at PR time. There is a Microsoft rule that PRs require at least 1 approver which we also have and isn't bypassable here, so effectively bypassing would be just for the PR rule and we can scope it to only a few people with it documented that it should only be used for document changes.

I'm good with either two. Thoughts?

Note: This one also has the same issue: #545

Is a third option to just run the same gates even for 'none code' PRs? If so, I think that is our least worst option. Yes, it means some unnecessary checks run, but it means that we don't have to worry about PRs being merged that have not cleared gates.

@bbonaby bbonaby mentioned this pull request Jun 23, 2026
7 tasks
@bbonaby

bbonaby commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

yea that's cool with me: Can you approve #549, merge it and then merge with main? That should get the pipelines running again.

Remove references to the TypeScript CLI from copilot-instructions.md
@MGudgin MGudgin force-pushed the user/gudge/remove_stale_ts_cli_refs branch from e4eff33 to e0c0ebf Compare June 23, 2026 14:48
@MGudgin MGudgin merged commit 17c333b into main Jun 23, 2026
18 checks passed
@MGudgin MGudgin deleted the user/gudge/remove_stale_ts_cli_refs branch June 23, 2026 15:20
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.

4 participants