Skip to content

Fix test fixture upgrade scripts #942

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 15 commits into from
Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@
"pojo",
"subword"
],
"files.eol": "\n"
"files.eol": "\n",
"typescript.tsdk": "node_modules/typescript/lib"
Copy link
Member

Choose a reason for hiding this comment

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

This is nice; let's upgrade Typescript in this PR as well though I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I upgraded TS as well, but I left the website at 4.5 as the typedocs only supports up to 4.5 and I didn't want to risk breaking the website!

Copy link
Member

Choose a reason for hiding this comment

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

Hmm I think that will be a problem, because we run typedocs on all the code in cursorless to generate api docs. We should prob pin both to the same version

When we switch to nx we'll only have one set of versions for everything fwiw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can try upgrade it but I'm not familiar with docasaurus and it's typedocs so I'd need someone to double check that. It may end up being a much bigger upgrade - if it does need more than a simple dependency upgrade do we want to just do that here or make a different PR for upgrading the website deps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I upgraded TS in website, it seems working to me as far as I can tell. The types are getting output in the contributor docs but I'm not sure they're 100% correct.

Copy link
Member

Choose a reason for hiding this comment

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

We should probably also make VSCode actually ask whether we want to use it too, otherwise nobody will actually switch to it.

Suggested change
"typescript.tsdk": "node_modules/typescript/lib"
"typescript.enablePromptUseWorkspaceTsdk": true,
"typescript.tsdk": "node_modules/typescript/lib"

Copy link
Member

@pokey pokey Sep 12, 2022

Choose a reason for hiding this comment

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

@auscompgeek that sounds reasonable to me. Any objections @SimeonC ?

Re types in website; I just kicked off a deploy preview. I'll take a look at docs once it's out to see if they look sane

Copy link
Member

Choose a reason for hiding this comment

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

Huh. Unfortunately this PR breaks the api docs. For example, in the docs for runCommand, see how on production there are a bunch of links in the description, but these seem to have disappeared for some reason in the deploy preview 😕

}
Loading