Skip to content

Moving to TS#47

Open
elazarcoh wants to merge 23 commits intojgclark:masterfrom
elazarcoh:to-ts
Open

Moving to TS#47
elazarcoh wants to merge 23 commits intojgclark:masterfrom
elazarcoh:to-ts

Conversation

@elazarcoh
Copy link
Copy Markdown
Collaborator

@elazarcoh elazarcoh commented Jan 13, 2022

Rewrite the extension in TS.
It's still a work in progress.

  • setup TS
  • make minimal changes to make it compile with TS
    • verify it works the same

@alkatar21
Copy link
Copy Markdown
Collaborator

I would suggest moving the refactoring to one or more separate PRs for traceability later?

Comment thread src/config.ts Outdated
@alkatar21
Copy link
Copy Markdown
Collaborator

Another suggestion from me would be to comment in the first line when something was generated and with which tool. Just for the traceability again.

@elazarcoh
Copy link
Copy Markdown
Collaborator Author

Another suggestion from me would be to comment in the first line when something was generated and with which tool. Just for the traceability again.

Not sure I'm following.
For now, only the config was auto generated. I intended it to be a one-time thing, not as part of the build pipeline.
It was more due to my laziness (🙃) of manually coping the configurations, rather than a sophisticated method to auto-generate code from the package.json...

It is possible to auto-generate it as part of the build pipeline, if we want to (in fact, I wrote a tool that does just that, just the other way around: TS -> package.json:contributions.configuration).

For now, we should continue to monitor any changes to the configuration section in the package.json, and update the TS config accordingly. If you see anything else missing, just let me know. Or better, create a PR to fix it (if possible, maybe to my fork?).

@alkatar21
Copy link
Copy Markdown
Collaborator

alkatar21 commented Jan 16, 2022

I did not mean it that way.
I guess config.ts and config-utils.ts are generated? My idea is to just put it in the first line as a comment, because at least for me it was unclear in the first moment how you came up with config-utils.ts.
I don't think it should be part of a pipline.

If you see anything else missing, just let me know. Or better, create a PR to fix it (if possible, maybe to my fork?).

Currently I'm waiting for all current PRs to be merged. After that I would set it up for me and try things out.

@elazarcoh
Copy link
Copy Markdown
Collaborator Author

I see.
I'm not sure considering it "auto-generated" is correct, if it's a one time, and passed a manual review.
In any case, the config.ts was auto-generated, and then modified manually (because it didn't get everything right).
For the config-utils.ts, I agree that I need to add some comments there. Probably, it would be clearer if I just create a concrete Configuration class, rather then using generic code (as it is now). I copied it from my previously mentioned tool.

Currently I'm waiting for all current PRs to be merged. After that I would set it up for me and try things out.

OK. don't wait for #45 though. I'll rewrite it with TS.

@alkatar21
Copy link
Copy Markdown
Collaborator

For the config-utils.ts, I agree that I need to add some comments there. Probably, it would be clearer if I just create a concrete Configuration class, rather then using generic code (as it is now). I copied it from my previously mentioned tool.

Then that would be fine for me. I don't care whether it's Generic or not, but it's currently not obvious to me what's happening and why. I would find it useful to improve that.

elazarcoh and others added 8 commits January 16, 2022 18:35
Bumps [terser](https://github.com/terser/terser) from 5.7.2 to 5.14.2.
- [Release notes](https://github.com/terser/terser/releases)
- [Changelog](https://github.com/terser/terser/blob/master/CHANGELOG.md)
- [Commits](https://github.com/terser/terser/commits)

---
updated-dependencies:
- dependency-name: terser
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…rser-5.14.2

Bump terser from 5.7.2 to 5.14.2
Bumps [nth-check](https://github.com/fb55/nth-check) from 2.0.0 to 2.1.1.
- [Release notes](https://github.com/fb55/nth-check/releases)
- [Commits](fb55/nth-check@v2.0.0...v2.1.1)

---
updated-dependencies:
- dependency-name: nth-check
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [ansi-regex](https://github.com/chalk/ansi-regex) from 3.0.0 to 3.0.1.
- [Release notes](https://github.com/chalk/ansi-regex/releases)
- [Commits](chalk/ansi-regex@v3.0.0...v3.0.1)

---
updated-dependencies:
- dependency-name: ansi-regex
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
…si-regex-3.0.1

Bump ansi-regex from 3.0.0 to 3.0.1
…h-check-2.1.1

Bump nth-check from 2.0.0 to 2.1.1
@elazarcoh elazarcoh marked this pull request as ready for review August 8, 2022 16:17
@elazarcoh
Copy link
Copy Markdown
Collaborator Author

Hi, getting back to this. So, I tested it on my machine and it works well. But I think it's better to have someone else test it as well. If anyone is willing, we can continue discussing it here for any bug or issue until we'll see it has none.

farwyler and others added 9 commits October 28, 2022 17:46
Support creating diagnostics for open files
Bumps [nanoid](https://github.com/ai/nanoid) to 3.3.3 and updates ancestor dependency [mocha](https://github.com/mochajs/mocha). These dependencies need to be updated together.


Updates `nanoid` from 3.1.20 to 3.3.3
- [Release notes](https://github.com/ai/nanoid/releases)
- [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
- [Commits](ai/nanoid@3.1.20...3.3.3)

Updates `mocha` from 8.4.0 to 10.1.0
- [Release notes](https://github.com/mochajs/mocha/releases)
- [Changelog](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md)
- [Commits](mochajs/mocha@v8.4.0...v10.1.0)

---
updated-dependencies:
- dependency-name: nanoid
  dependency-type: indirect
- dependency-name: mocha
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [markdown-it](https://github.com/markdown-it/markdown-it) to 12.3.2 and updates ancestor dependency [vsce](https://github.com/Microsoft/vsce). These dependencies need to be updated together.


Updates `markdown-it` from 10.0.0 to 12.3.2
- [Release notes](https://github.com/markdown-it/markdown-it/releases)
- [Changelog](https://github.com/markdown-it/markdown-it/blob/master/CHANGELOG.md)
- [Commits](markdown-it/markdown-it@10.0.0...12.3.2)

Updates `vsce` from 1.96.3 to 2.13.0
- [Release notes](https://github.com/Microsoft/vsce/releases)
- [Commits](microsoft/vscode-vsce@v1.96.3...v2.13.0)

---
updated-dependencies:
- dependency-name: markdown-it
  dependency-type: indirect
- dependency-name: vsce
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
…noid-and-mocha-3.3.3

Bump nanoid and mocha
…rkdown-it-and-vsce-12.3.2

Bump markdown-it and vsce
Correct small typo on README.md
delete dist/* files from repository.
(The comments here have gone onto other things, but I'm merging this original small change.)
@alkatar21
Copy link
Copy Markdown
Collaborator

@elazarcoh As some PRs got merged this maybe needs some updates too?

@elazarcoh
Copy link
Copy Markdown
Collaborator Author

@elazarcoh As some PRs got merged this maybe needs some updates too?

Done.
Do we want to merge it like this, without further testing? Maybe we can make a pre-release version for testing? @jgclark what do you think?

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.

5 participants