release: Add sanity checks at beginning of each function #464
Conversation
| @@ -273,6 +285,9 @@ as_release_set_version (AsRelease *release, const gchar *version) | |||
| gint | |||
| as_release_vercmp (AsRelease *rel1, AsRelease *rel2) | |||
| { | |||
| g_return_val_if_fail (AS_IS_RELEASE (rel1), 0); | |||
| g_return_val_if_fail (AS_IS_RELEASE (rel2), 0); | |||
There was a problem hiding this comment.
Those two checks aren't needed, as as_release_get_version already verifies the input.
There was a problem hiding this comment.
It still helps the consumer to know that it is the content that they passed to vercmp that is wrong and also to know if it is rel1 or rel2
|
Thanks! That must have been quite a bit of work to add these - some newer public API has these checks, while they are generally omitted from private and internal API (with some exceptions) as well as older public API (the latter because of laziness, old conventions, and the thought that the checks won't be needed). |
Allows to fail with a loud message on what happened before an actual issue.
|
For context, we had a crash in AppCenter on our launch day elementary/appcenter#1993 which could have just been a warning with this MR 🙂 |
I assumed there was a story like this behind this PR :-D |
Allows to fail with a loud message on what happened before an actual issue.