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

Update BackupConfig.java #236

Merged
merged 4 commits into from
Apr 27, 2024
Merged

Update BackupConfig.java #236

merged 4 commits into from
Apr 27, 2024

Conversation

SirDank
Copy link
Contributor

@SirDank SirDank commented Apr 27, 2024

including sample files / folders should have been set to false by default

backup: 
  include: 
    # Add specific files or folders you want to include in the backup, to the list below.
    # Windows/Linux formats are supported. './' stands for the servers root directory.
    enable: false

@Osiris-Team
Copy link
Owner

The examples contain "./", thus by having that enabled it backs up the current working directory by default before executing any tasks, which should be the default in my eyes, to ensure that even if autoplug breaks something you can always recover.

Yes you get some warnings for the other examples, but thats a small price to pay.

@SirDank or do you have other arguments against this?

@SirDank
Copy link
Contributor Author

SirDank commented Apr 27, 2024

My bad, did not know it worked that way.
I have made some changes to better handle this ( not sure if it's entire correct but if not, the comments under backup_include_list could be moved to backup_include)

@Osiris-Team
Copy link
Owner

@SirDank thats better, however since there now is only one value, the yaml parser will change

include-list:
  - ./

to

include-list: ./

which makes this config section harder to understand.
Im dont think Dyml (yaml parser) has something to force a list display of values. Maybe a PR there for this would be needed then.

@SirDank
Copy link
Contributor Author

SirDank commented Apr 27, 2024

Added a simple work-around by including server.properties

@Osiris-Team Osiris-Team merged commit fb15316 into Osiris-Team:master Apr 27, 2024
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