refactor: no revert on revoke

[MerlinEgalite]

Apply @QGarchery's suggestion: https://github.com/morpho-org/metamorpho/pull/353/files#r1436881439

[MathisGD]

Great! Just I think that we should adapt this naming (prop: revokePendingXIfAny)

[MerlinEgalite]

Great! Just I think that we should adapt this naming (prop: revokePendingXIfAny)

revokePendingXIfAny suggests that the logic changes depending on the case though..

[Rubilmax]

It is fine to me to keep the current naming because they are all consistent, but this behavior (that goes against our conventions for software design) should be documented in the natspecs

Something like:
/// @dev This function is guaranteed to not revert.

[StErMi]

I understand the reason for the change and it's not a big deal.
The question is: wouldn't be better to leave there the if statement and simply return instead of performing a delete + emit when there's nothing to be revoked?

Leaving aside the average gas cost of the operation in a normal scenario, I would consider not to emit an event when you have not revoked any proposal (because there was any).

[QGarchery]

I would consider not to emit an event when you have not revoked any proposal (because there was any).

I don't think it will be problematic for integrators to have those useless events. So the tradeoff is between event readability and code readability, and I'm in favor of code readability (so the current solution without the if statement)