Skip to content
This repository was archived by the owner on Oct 11, 2023. It is now read-only.

Fix: handle errors on 'merge' instead of 'merge_allowed'#292

Merged
joao-paulo-parity merged 1 commit intomasterfrom
merge_failure
May 27, 2021
Merged

Fix: handle errors on 'merge' instead of 'merge_allowed'#292
joao-paulo-parity merged 1 commit intomasterfrom
merge_failure

Conversation

@joao-paulo-parity
Copy link
Contributor

It came to me hours after implementing #291 that the error handling had been terribly misplaced. It's merge that might return a MergeFailureWillBeSolvedLater, not merge_allowed.

This kind of mistake is not easy to prevent at the moment because every error variant is jammed into the big dreadful Error enum, thus there's no help at compile time for narrowing specific errors to handle for specific functions - you basically have to read their implementation to know which errors are relevant for them. It certainly should be improved in the future. There are widely-known Rust practices for preventing this kind of "code smell".

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant