build/docs: Updated ttypescript version #5767
Conversation
…d contributing.md
|
@Symulation12 Thanks for your contribution! 🌵🚀 |
Symulation12
changed the title
Symulation12
changed the title
Symulation12
changed the title
CONTRIBUTING.md
Outdated
| npm install -g typescript | ||
| npm install -g ts-node | ||
| npm install -g typescript@4.6.4 | ||
| npm install -g ts-node@10.8.0 |
There was a problem hiding this comment.
One worry I have here is these getting out of date. It's going to be extremely likely that somebody will forget to bump this documentation in the future. This seems like the same problem, but in reverse. People will install this old version and then it won't work with whatever the new version needs.
I think this is more likely of a problem than the one you ran into, so I'd prefer not to change the versions.
(Happy to discuss more too, or maybe there's some solution to keep this doc up to date or a command that installs the package.json version globally that I don't know about?)
There was a problem hiding this comment.
Ah, true, that is something I should have considered. I looked around to see if I could find a better solution, but I couldn't find anything cleaner than simply listing the most recent version. Putting it that way, I definitely agree that leaving it out is the better option for now.
README.md
Outdated
| @@ -242,7 +242,7 @@ instead of changing your local cactbot files. | |||
|
|
|||
| To install npm and start Webpack, follow these steps: | |||
|
|
|||
| 1. Install [nodejs and npm](https://nodejs.org/en/download/) | |||
| 1. Install [nodejs 18.17.1 and npm 9.6.7](https://nodejs.org/en/download/) | |||
There was a problem hiding this comment.
| 1. Install [nodejs 18.17.1 and npm 9.6.7](https://nodejs.org/en/download/) | |
| 1. Install [nodejs 18.17.1 and npm 9.6.7](https://nodejs.org/en/download/) |
This feels like the same issue here as above. Re: nodejs, I feel like cactbot should always work against the LTS version of nodejs and if it doesn't then it's a bug to be fixed. As soon as 18.17.1 is not the most current LTS version, then it's going to be much harder to find as well and folks will have to dig around in versions to get to it.
I'd rather make a note that says "cactbot should always work with the latest LTS and file an issue if it doesn't".
I would also be ok if it said 18.x.x LTS release too rather than specifically 18.17.1 and the npm version.
There was a problem hiding this comment.
Fair point, that is definitely a more sensible solution. Given that goal, then just specifying that it should be the LTS release should be sufficient.
|
Also also, thanks for testing this and the update! Glad this was an easy fix. |
|
Of course! |
quisquous
changed the title
quisquous
changed the title
|
Thank you!! |
Tested with a fresh environment on node 18.17.1. As was mentioned in #5764 , I also added version numbers to the recommended global packages in CONTRIBUTING (the same ones as in package.json), and version numbers to the README for nodejs and npm (current LTS).
Apologies if I used the wrong label for this PR.