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

Remove "config.json" as a tracked respository file, rename to "config.json.example" to highlight the config file requires manual config. #275

Closed
DrKittens opened this issue Jan 22, 2023 · 12 comments
Labels
core feature request Feature request related to the core SquadJS API

Comments

@DrKittens
Copy link
Contributor

DrKittens commented Jan 22, 2023

What is the issue, e.g. map voting, you would like to solve?

git pull replaces config.json with the example config by default.
git pull should not replace config.json and the provided config.json should be labelled as an example
Renaming config.json and removing the tracking of the file from git allows for a persistant configuration between updates of master without needing to copy/replace the configured config.json file.
Cant see anything in the build / install files as discussed in #273 as been problematic or altering to the config.json file.

How would you like us to solve this issue?

See #273.

For a new user basing on that commit the following update workflow is functional:

  1. git clone https://github.com/Team-Silver-Sphere/SquadJS.git
  2. Copy config.json.example to config.json
  3. configure config.json
  4. install with yarn install
  5. (optional) git rm config.json (tells git to ignore the newly created file, possibly mandatory if your git blocks the next pull due to un-commited changes, ie it finds a new file you haven't told it about with git add <file> for some reason)
  6. run with node index.js

Same steps as previous with the additional step of acknowledging the shipped config file is unconfigured and needs manual intervention.

Once installed the follow workflow can be used to update the install from git without dropping config & run the tool:

  1. git pull (no longer replaces config.json because its called called config.json.example upstream)
  2. yarn install
  3. node index.js

Additional Info

Add any other info or screenshots about the feature request here.
I've had this in my live branch for a few updates cycles and its functional
Added @ c825cf1

Since then i've updated twice (thanks ect0s) and rebased against master, config.json has persisted the entire time.

@DrKittens DrKittens added the core feature request Feature request related to the core SquadJS API label Jan 22, 2023
@DrKittens DrKittens changed the title Remove "config.json" as a tracked respository file Remove "config.json" as a tracked respository file, rename to "config.json.example" to highlight the config file requires manual config. Jan 22, 2023
@ect0s
Copy link
Contributor

ect0s commented Jan 22, 2023

const configPath = path.resolve(__dirname, '../config.json');

This would need to be edited in your PR.

@werewolfboy13
Copy link
Collaborator

werewolfboy13 commented Jan 22, 2023

Still not a valid fix. There is a job "yarn build all" that overrides these changes. A solution for this must be found first.

Also templates will need to edited as well.

See my comments and take action on this and your pull request before we consider this.

@ect0s
Copy link
Contributor

ect0s commented Jan 22, 2023

Still not a valid fix. There is a job "yarn build all" that overrides these changes. A solution for this must be found first.

Also templates will need to edited as well.

Does yarn buildall Call the BuildConfig function within the Factory I linked?

Because changing the path there would then solve the issues with yarn build all.

"build-all": "node squad-server/scripts/build-config-file.js && node squad-server/scripts/build-readme.js"

SquadServerFactory.buildConfigFile()

I'm pretty sure thats all that would need to change.

@werewolfboy13
Copy link
Collaborator

Still not a valid fix. There is a job "yarn build all" that overrides these changes. A solution for this must be found first.

Also templates will need to edited as well.

Does yarn buildall Call the BuildConfig function within the Factory I linked?

Because changing the path there would then solve the issues with yarn build all.

Should, I can double check later. He still has the templates that need to be corrected before push can be approved.

@ect0s
Copy link
Contributor

ect0s commented Jan 22, 2023

@DrKittens

Looked at this briefly;

Werewolfboy is partially correct, but the TLDR is:

Make your changes to the readme-template;

https://github.com/Team-Silver-Sphere/SquadJS/blob/master/squad-server/templates/readme-template.md

Then:

const configPath = path.resolve(__dirname, '../config.json');

Change this to point to your config.json.example.

Then:

Add config.json to the .gitignore

And finally, test via running yarn build-all

The config file and readme are regenerated by the above commands, this is so things like plugin options and defaults are automatically regenerated into the readme/config file without manually intervention, via the static methods get optionsSpecification() and get description()

@ect0s
Copy link
Contributor

ect0s commented Jan 22, 2023

I'm not sure what, if anything else would need doing past that, would be a question for @Thomas-Smyth but I think the above covers most of it.

I'm just not entirely sure if theres anything in the github actions side that would also need work.

@werewolfboy13
Copy link
Collaborator

@ect0s you need to delete the config, then do the .gitignore. Two separate pushes, saying from experience.

@werewolfboy13 werewolfboy13 linked a pull request Jan 23, 2023 that will close this issue
@reck1610
Copy link
Contributor

I would suggest changing the name to config.example.json in order to keep the file extension.

This would work great for multi server setups too.
The config files for multiple servers could be stored like this:

config.example.json
config.testserver.json
config.server1.json
config.server2.json

All config files except config.example.json should be ignored in this case.
.gitignore should look like this:

config.*.json
!config.example.json

@ect0s
Copy link
Contributor

ect0s commented Jan 24, 2023

Yeah, that would work.

As is, we already support multiple configs via node index.js configname.json

@DrKittens
Copy link
Contributor Author

DrKittens commented Jan 24, 2023

Will update with ammendments posted in this thread when work stops kicking my butt.

Thanks for clarifying and elaborating on what i'd missed ect0s & werewolfboy

@werewolfboy13
Copy link
Collaborator

@DrKittens Checking in, seeing if you are still wanting to push your updates.

@werewolfboy13
Copy link
Collaborator

No activity on ticket. Closing as stale.

@werewolfboy13 werewolfboy13 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core feature request Feature request related to the core SquadJS API
Projects
None yet
4 participants