Skip to content
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

chore: remove unnecessary TypeScript as #220

Merged
merged 3 commits into from Dec 12, 2023

Conversation

boyum
Copy link
Contributor

@boyum boyum commented Dec 11, 2023

I read through some of the code and found some TypeScript as statements that seemed unnecessary. You'll of course decide for yourself if you want to do this change. No changes were done to the runtime JS code.

@hosseinmd
Copy link
Owner

LGTM, but actions failed, I don't know why. Also, I fixed one of them.

@boyum
Copy link
Contributor Author

boyum commented Dec 12, 2023

Thank you. I don't know either, they seem to work fine locally. It also seems to be an issue in master https://github.com/hosseinmd/prettier-plugin-jsdoc/actions/runs/7180581677/job/19553161503.

@danielpza
Copy link
Collaborator

It's because of an update in prettier, https://prettier.io/blog/2023/11/13/curious-ternaries

@danielpza
Copy link
Collaborator

Hmm, this is weird, the github actions workflow file says to try installing a 3.0.x prettier version, but the one that got installed was 3.1.1 which includes the new formatting changes:

https://github.com/hosseinmd/prettier-plugin-jsdoc/actions/runs/7180599499/workflow?pr=220#L17
image

https://github.com/hosseinmd/prettier-plugin-jsdoc/actions/runs/7180599499/job/19553214222?pr=220#step:6:16
image

@hosseinmd
Copy link
Owner

Are there any options for disabling that?

@danielpza
Copy link
Collaborator

danielpza commented Dec 12, 2023

I just noticed that yarn is installing prettier@3 instead of prettier@3.0. This is likely due to how yaml treats strings and numbers. Perhaps changing the github actions matrix for the prettier version to be explicitly an string would fix it:

https://github.com/hosseinmd/prettier-plugin-jsdoc/blob/2d7008fc968ef1afba4bb0bb94ac3ad4010ad00d/.github/workflows/test.yml#L17C7-L17C7

-        prettier: [3.0]
+        prettier: ["3.0"]

@danielpza
Copy link
Collaborator

See #221

@hosseinmd
Copy link
Owner

Merged, Thanks.

@hosseinmd
Copy link
Owner

The presence of this issue in Prettier has unfortunately hindered our ability to test several older versions.

@boyum
Copy link
Contributor Author

boyum commented Dec 12, 2023

Nice ✨

@hosseinmd hosseinmd merged commit 18dff67 into hosseinmd:master Dec 12, 2023
3 checks passed
@boyum boyum deleted the chore/remove-unnecessary-ts-as branch December 12, 2023 15:37
@hosseinmd
Copy link
Owner

Thank you.

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.

None yet

3 participants