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

Add json configuration ad email notification #33

Merged
merged 4 commits into from
Jul 25, 2024

Conversation

giuseongit
Copy link
Contributor

Hi! First of all, geat project, I just made some small additions to the program which are not exaustive and are considered to be just a small step towards a better project. Here's what I did:

JSON configuration

I added the possibility to pass all the configuration as a json file. This comes a long a very simple configuration management, which can be used as a foundation for further developments.

removed some panic()

Calling the panic() function, actually stops the execution of the program, wherever it is. This makes difficult to correctly handle errors (more infos in the commit message of ace6224fa8311850392be23e06f25fb80be8950b)

mail notifications

backups are often an automated and unsupervised task so it is useful some sort of notification in order to know if the backup was successful.

Please consider (and review) this PR, I'm willing to discuss every aspect of it.

During backup there are multiple operations that may fail which were ignored.
This commit begins to address this issue by forwarding the errors back to the main function
that can tell the user if the backup was successful or not.
Since `panic()` stops the execution of the program, by returning the error
to upper levels we can enable more sophisticated notification modes without
worring updating every single function that may fail.
Copy link
Owner

@tizbac tizbac left a comment

Choose a reason for hiding this comment

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

Nice work , thanks

@tizbac tizbac merged commit 06fdf7c into tizbac:master Jul 25, 2024
@dalf dalf mentioned this pull request Sep 14, 2024
@giuseongit giuseongit deleted the enhances branch September 16, 2024 10:14
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