pmatilai commented on this pull request.


> @@ -274,17 +414,22 @@ static rpmRC buildSpec(BTA_t buildArgs, rpmSpec spec, 
> int what)
 exit:
     free(cookie);
     spec->rootDir = NULL;
-    if (rc != RPMRC_OK && rpmlogGetNrecs() > 0) {
+    if (rc != RPMRC_OK && rc != RPMRC_MISSINGBUILDREQUIRES &&
+       rpmlogGetNrecs() > 0) {
        rpmlog(RPMLOG_NOTICE, _("\n\nRPM build errors:\n"));

The if-condition continues at the same indentation level as the actual if-block 
itself, so its really easy to mistake the rpmlogGetNrecs()  to be part of the 
if-block rather than the condition. This is a recurring theme in the patches, 
please fix all such cases. Multiple options how to deal with it:
a) indent the continued condition to clearly deeper level (iirc this is what 
```indent -kr``` does)
b) find a way to make the condition fit on one line, using helper variables and 
such as necessary
c) put the opening curly brace on a line of its own - its a kind of style 
violation but it does make the construct obvious

With meaningful names for helper variables, b) typically produces by the most 
readable code.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/593#pullrequestreview-233993450
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Reply via email to