After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 749035 - use g_assert() instead of g_assert_cmpint() in src/ and libide/
use g_assert() instead of g_assert_cmpint() in src/ and libide/
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-05-06 18:37 UTC by Christian Hergert
Modified: 2015-05-07 09:00 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Replace g_assert_cmpint and g_assert_cmpstr with g_assert (18.86 KB, patch)
2015-05-07 08:09 UTC, Dimitris Zenios 		none Details | Review
Replace g_assert_cmpint and g_assert_cmpstr with g_assert (4.58 KB, patch)
2015-05-07 08:35 UTC, Dimitris Zenios 		committed Details | Review

Description Christian Hergert 2015-05-06 18:37:12 UTC 
g_assert_cmpint() does not get compiled out when G_DISABLE_ASSERT is defined, as it is part of the unit testing framework.

We need to use g_assert() in src/ and libide/ directories as we want all assertions to compile out in optimized builds.

`git grep g_assert_cmpint` should help you find all the locations.

Possibly check for g_assert_cmpstr() too, there may be a few floating around.
Comment 1 Dimitris Zenios 2015-05-07 07:33:59 UTC 
What about tests and egg folder?Those should be done to?
Comment 2 Dimitris Zenios 2015-05-07 08:09:48 UTC 
Created attachment 303012 [details] [review]
Replace g_assert_cmpint and g_assert_cmpstr with g_assert

I replaced the two functions with g_assert also in egg and test folder.
Comment 3 Christian Hergert 2015-05-07 08:23:16 UTC 
Review of attachment 303012 [details] [review]:

First part looks good.

We do want to continue using g_assert_cmpint() inside of tests/ though. (It has g_print() helpers to see what the failed value which is useful when running unit tests).
Comment 4 Dimitris Zenios 2015-05-07 08:35:32 UTC 
Created attachment 303015 [details] [review]
Replace g_assert_cmpint and g_assert_cmpstr with g_assert

Added a new patch that removed the tests/ changes
Comment 5 Christian Hergert 2015-05-07 08:59:33 UTC 
LGTM, Thanks!

Attachment 303015 [details] pushed as 43fc972 - Replace g_assert_cmpint and g_assert_cmpstr with g_assert