Laravel-Backpack / BackupManager

Admin interface for managing database and file backups in Backpack for Laravel
http://backpackforlaravel.com
Other
333 stars 93 forks source link

[New version] Moving configs to separate backupmanager file, remove old dependencies #102

Closed pxpm closed 2 years ago

pxpm commented 2 years ago

WHY

BEFORE - What was wrong? What was happening before this PR?

This aims to fix https://github.com/Laravel-Backpack/BackupManager/pull/101 and also plan the major version with the breaking changes discussed here: https://github.com/Laravel-Backpack/BackupManager/issues/68

AFTER - What is happening after this PR?

This should be tagged as a NEW MAJOR VERSION, for L9+, spatie 8+.

I've refactored the config, so we publish both configs config/backup.php (spatie), config/backpack/backupmanager.php (backpack).

I've introduced also https://github.com/Laravel-Backpack/BackupManager/pull/84 here.

Added the flags option, technically we were already proving a config for it backpack_flags but never using it in backup:run.

I accounted for the fact that previous users migth have their configs inside that old config in config/backup.php and make it backwards compatible there.

Added a new notification that triggers imedially when the user press the Backup button, and subsquently we show the result of the ajax request with other notification.

I cleaned the code here and there. From my tests everything seems to be working, willing to give this a try @tabacitu ?

promatik commented 2 years ago

@pxpm does this invalidates #101?

pxpm commented 2 years ago

@pxpm does this invalidates #101?

Yes @promatik just didn't closed as I am not sure how @tabacitu want to do here.

tabacitu commented 2 years ago

I love this! ❀️ It fixes something that was a problem for a loooong time in this repo (the config). @promatik please double-check this, collaborate until it's ready to merge and tag as a new version, from both your points of view. The new release is in your hands @pxpm and @promatik πŸ’ͺ Don't go overboard, of course. This is a small niche package that only does one thing. And it should stay that way.

You guys let me know when this is ready to merge! πŸ™ (I love saying that btw πŸ˜…)

PS. The only thing I notice right now is that the new config works differently from the old config, so that means we should have an upgrade guide in the README.

promatik commented 2 years ago

@pxpm, @tabacitu, I've made a solution to fix the bad UX. It's in a separate PR, #103. Please let me know your thoughts.

promatik commented 2 years ago

Since this will be a BC, we may also fix https://github.com/Laravel-Backpack/BackupManager/pull/87 πŸ™Œ

tabacitu commented 2 years ago

@promatik is this ready from your point of view? Does it have your thumbs up? I want to click Merge but I'll only do it after I know you've also tested and reviewed this.

promatik commented 2 years ago

@tabacitu this one is ready, but with the UX issues fixed on #103 πŸ˜…

welcome[bot] commented 2 years ago

WHOOP-WHOOP! Congrats, your first PR on this repo has officialy been merged. party If you want to help out the community in other ways, you can:

Again. Thank you for the PR. You are a wonderful person. Keep 'em coming :-) Cheers! -- Justin Case The Backpack Robot P.S. Help in the Backpack community is rewarded with free Backpack commercial licenses. It's the least we can do. If you feel you've helped the community with PRs, help & other stuff, please shoot Tabacitu an email and ask him if you qualify for free licenses. You scratch my back, I scratch your back. Thank you!