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

Replace exit() call with thrown exceptions #141

Merged
merged 3 commits into from
Jun 10, 2023
Merged

Replace exit() call with thrown exceptions #141

merged 3 commits into from
Jun 10, 2023

Conversation

Azhng
Copy link
Contributor

@Azhng Azhng commented Aug 18, 2022

Description

Previously, exit() function was called when popsift encountered errors
(e.g. out of memory). This error handling made graceful error handling
within the application using popsift difficult.
This commit replaces all the exit() call with throwing a runtime error
instead.

Features list

@Azhng Azhng changed the title Replace exit() call with thrown exceptions WIP: Replace exit() call with thrown exceptions Aug 18, 2022
@Azhng Azhng marked this pull request as draft August 18, 2022 17:46
@Azhng Azhng changed the title WIP: Replace exit() call with thrown exceptions [WIP]: Replace exit() call with thrown exceptions Aug 18, 2022
@Azhng Azhng changed the title [WIP]: Replace exit() call with thrown exceptions Replace exit() call with thrown exceptions Aug 18, 2022
@Azhng Azhng marked this pull request as ready for review August 18, 2022 17:47
@Azhng
Copy link
Contributor Author

Azhng commented Aug 29, 2022

@simogasp Let me know what you think :D

@simogasp simogasp added this to the v1.0.0 milestone Aug 30, 2022
@simogasp simogasp requested a review from griwodz August 30, 2022 09:19
@simogasp
Copy link
Member

simogasp commented Sep 3, 2022 •
edited
Loading

Thanks for this!
Maybe we could just replace the code that you modified with a proper call to POP_FATAL(s) since we have it and it does the same job.
What do you think @griwodz ?

@Azhng
Replace exit() call with thrown exceptions
f872278 
Address alicevision#139

Previously, exit() function was called when popsift encountered errors
(e.g. out of memory). This error handling made graceful error handling
within the application using popsift difficult.
This commit replaces all the exit() call with throwing a runtime error
instead.
@Azhng
Copy link
Contributor Author

Azhng commented Sep 21, 2022

Good point, updated the PR.

simogasp
simogasp approved these changes Jun 9, 2023
View reviewed changes
Copy link
Member

@simogasp simogasp left a comment

Choose a reason for hiding this comment

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

LGTM!

@simogasp simogasp modified the milestones: v1.0.0, v0.9.1 Jun 9, 2023
@simogasp simogasp merged commit 4b4b247 into alicevision:develop Jun 10, 2023
@simogasp
Copy link
Member

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simogasp simogasp simogasp approved these changes

@griwodz griwodz Awaiting requested review from griwodz

Assignees
No one assigned
Labels
refactoring type:enhancement
Projects
None yet
Milestone
v0.9.1
Development

Successfully merging this pull request may close these issues.
2 participants
@Azhng @simogasp