Skip to content

JS-690 Update message on latest supported Typescript #5303

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 1 commit into from
Apr 16, 2025
Merged

Conversation

vdiez
Copy link
Contributor

@vdiez vdiez commented Apr 15, 2025

JS-690

This will happen again every time renovate updates TS version. We should get rid of this TS hardcoded version in Java side

@vdiez vdiez requested a review from zglicz April 15, 2025 15:06
@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title Update message on latest supported Typescript JS-690 Update message on latest supported Typescript Apr 15, 2025
Copy link

Copy link
Contributor

@zglicz zglicz left a comment

Choose a reason for hiding this comment

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

So can we do it better? If we keep this on the Java side it should be either verified or filled during the build process. Or do you want to wait until we migrate to analyzerProject.ts?

@vdiez
Copy link
Contributor Author

vdiez commented Apr 16, 2025

I think we can do the same log catching the exception in the Node.js process and logging this same message. Not sure it's worthy overengineering the whole thing to have the semver string on the java side. This is just a fix to have this matching in the next release, but I would like to remove it completely from Java

@zglicz zglicz marked this pull request as ready for review April 16, 2025 09:23
Copy link
Contributor

@zglicz zglicz left a comment

Choose a reason for hiding this comment

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

Create a ticket to move it to analyzeProject.ts under your master mmf? Or is this verified by an integration test?

@vdiez vdiez merged commit 5b7f9bc into master Apr 16, 2025
24 checks passed
@vdiez vdiez deleted the update-ts-583 branch April 16, 2025 09:35
@vdiez
Copy link
Contributor Author

vdiez commented Apr 16, 2025

no, it's not. just from a unit test. Ticket created: https://sonarsource.atlassian.net/browse/JS-692

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.

2 participants