[Rpm-maint] [rpm-software-management/rpm] Dynamic Build Dependencies (#593)

[Panu Matilainen]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20190506/26b1f5fc/attachment.html>