Skip to content

J sre 2742 Convert to typescript #10

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 26 commits into from
Mar 6, 2025

Conversation

d10zero
Copy link

@d10zero d10zero commented Feb 14, 2025

What

Replaces the cdktf-diff SHELL logic within the action into typescript.

Why

Logic is getting too complex to keep in shell. Logic in typescript will also allow us to write extensive test coverage.

How

Move logic into typescript and adds 100% tests coverage!

@d10zero d10zero changed the title J sre 2742 initial js ts project setup J sre 2742 Convert to typescript Feb 25, 2025
@d10zero d10zero requested review from whatisdot February 27, 2025 20:42
@whatisdot
Copy link
Contributor

Can you add a .nvmrc file that pins the node version? It would make things just a little bit easier, because we won't have to remember to switch node versions.

@d10zero
Copy link
Author

d10zero commented Feb 27, 2025

Can you add a .nvmrc file that pins the node version? It would make things just a little bit easier, because we won't have to remember to switch node versions.

@whatisdot there is a .nvmrc file in this PR already !

whatisdot
whatisdot previously approved these changes Feb 27, 2025
Copy link
Contributor

@whatisdot whatisdot left a comment

Choose a reason for hiding this comment

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

I've tested this, and can confirm it works as expected 👍

Copy link
Contributor

@whatisdot whatisdot left a comment

Choose a reason for hiding this comment

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

I've re-run my tests and can confirm that this still works as expected.

This was partly a venture into using LLMs to generate a solution. While it is impressive, it's still lower quality than the work I've seen from @d10zero when she implements without AI assistance. This might be part of the "burgeoning churn" that this paper is referring to:

In 2022-2023, the rise of AI Assistants are strongly correlated with "mistake code"
being pushed to the repo.
...(the correlation between revisions needed and Copilot usage) have grown in tandem.

This probably means we need to be more critical of AI assistance/suggestions before we submit code for review.

@d10zero d10zero changed the base branch from main to 0.3.0-rc March 6, 2025 17:07
@d10zero d10zero merged commit 8932164 into 0.3.0-rc Mar 6, 2025
1 check passed
@d10zero d10zero deleted the jSRE_2742-initial-js-ts-project-setup branch March 6, 2025 17:11
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.

3 participants