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.

Sign up for GitHub

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

Jump to bottom

add BackupStarted event #596

Closed
wants to merge 1 commit into from
Closed

add BackupStarted event #596

wants to merge 1 commit into from

Conversation

ctf0
Copy link

@ctf0 ctf0 commented Jan 7, 2018

u can use it like

Event::listen('Spatie\Backup\Events\BackupStarted', function () {
    return logger(‘db backup has started’);
});

#594

@ctf0
add BackupStarted event
17ea0e7 
u can use it like
```php
Event::listen('Spatie\Backup\Events\BackupStarted', function () {
    return logger(‘db backup has started’);
});
```
@introwit
Copy link
Contributor

introwit commented Jan 8, 2018

Hey thanks for the PR :) Could you please also add the tests too?

@ctf0
Copy link
Author

ctf0 commented Jan 8, 2018

sorry, i dont know how to write tests, however the pr isnt that big, its just an event that fires b4 the backup start

freekmurze
freekmurze requested changes Jan 8, 2018
View reviewed changes
Copy link
Member

@freekmurze freekmurze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make the event more useful by letting it hold a BackupJob. Also, fire off the event in the run function of BackupJob.

@ctf0
Copy link
Author

ctf0 commented Jan 8, 2018

@freekmurze am not sure what u r after, why we need the BackupJob ?

@freekmurze
Copy link
Member

freekmurze commented Jan 8, 2018 •
edited
Loading

It would be nice of the consumer of the event to be able to know which backup is starting.

@ctf0
Copy link
Author

ctf0 commented Jan 8, 2018 •
edited
Loading

actually this was more intended for the client side alert, to inform the user that no writing ops is currently available, but okay, i can make it have the same footprint as BackupWasSuccessful what u think ?

@freekmurze
Copy link
Member

Let the event hold BackupJob

@ctf0
Copy link
Author

ctf0 commented Jan 16, 2018 •
edited
Loading

apologize for the delay, but i found i can achieve what am after through https://laravel.com/docs/5.5/scheduling#task-hooks so no need for this event

also check laravel/framework#20298

@ctf0 ctf0 closed this Jan 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@freekmurze freekmurze freekmurze requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ctf0 @introwit @freekmurze