Skip to content

Conversation

th1000s
Copy link
Collaborator

@th1000s th1000s commented Oct 17, 2021

Now process::exit is only called in four places.

Plus some small improvements.

time,
line_number,
code,
})
Copy link
Owner

Choose a reason for hiding this comment

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

Ah, thanks! I had not internalised that ? could be used on Option. I had thought of it only in the context of Result (error-handling in general is an area in which the delta codebase could be improved -- I haven't dealt properly with situations that involve different Result types.)

Copy link
Owner

@dandavison dandavison left a comment

Choose a reason for hiding this comment

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

LGTM!

Is there any reason not to use this in the subcommands/diff module and eliminate the config.error_exit_code variable? I'll open a PR for that unless it turns out that I can see why you didn't do it.

EDIT: OK, I see subcommands/diff doesn't actually exit, it just computes appropriate integer exit codes, so it's less obvious how/whether to refactor.

@dandavison
Copy link
Owner

Thanks a lot for this clean up. It is of course an important part of the public API as well as being a codebase improvement.

@dandavison
Copy link
Owner

Rebased on master to resolve conflict

@dandavison dandavison merged commit 4f5e3eb into dandavison:master Oct 18, 2021
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