search

Team-Silver-Sphere / SquadJS

Squad Server Script Framework
Boost Software License 1.0
163 stars 123 forks source link

docs: update templates. #365

Closed Ulibos closed 1 month ago

Ulibos commented 2 months ago

Templates were out of date which was causing issues when commiting with husky on (they were lacking new 'host' parameter for ftp config). It also standardizes newlines in those files which generally makes git and other tools (linters) happier.

Ulibos commented 2 months ago

Not suited for merge yet, needs a couple amendments.

Ulibos commented 2 months ago

Ok, now it's done.

Thomas-Smyth commented 1 month ago

As per the code linked below, the FTP host parameter is optional. Furthermore, in the majority of cases, it will not be required as the FTP host will be the same as the server host. https://github.com/Team-Silver-Sphere/SquadJS/blob/master/squad-server/index.js#L185

Please refer to my comment on changes to timestamps being off by default in the comment linked below. https://github.com/Team-Silver-Sphere/SquadJS/pull/327#issuecomment-2236594007

Given this, I do not see much benefit in these changes. Thus, I will file this pull request, however, if you disagree with my comments please let me know and I will re-review them.

Ulibos commented 1 month ago

@Thomas-Smyth Well, some of those changes (host option) are already present in current docs but are not in templates. Which causes builds with husky to wipe those out each build and seems overall a strange solution. As for optional timestamps and host - Imo they still should be present in the docs. Right now getting familiar with squadjs is hard because a lot of stuff is documented in a terse way obvious only to those who have wrote or at least have extensively read the sources. Introducing those optionals to make people's life easier is still necessary. For now we are relying on trible knowledge a bit too much. If you have in mind a specific way of introducing such optionals then let me know and I will make a new PR with those in mind.

Thomas-Smyth commented 1 month ago

I am happy for them to be added to the readme but they should not be added to the config. The documentation should also include the default values.

Ulibos commented 1 month ago

Ok, got you. I will address this later then. I'm planning to include those in documentation both as text and as a config example. It's just the original config that I'm not going to add them to.