Re: [Rpm-maint] [rpm-software-management/rpm] Implement %elif (#710)
Ok, Rebased and merged. -- 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/710#issuecomment-496093898___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Implement %elif (#710)
Closed #710. -- 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/710#event-2368433852___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Implement %elif (#710)
Corrected the commit message of the first commit. -- 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/710#issuecomment-495102226___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Implement %elif (#710)
pavlinamv commented on this pull request.
> rl->lastConditional = lineType;
spec->readStack = rl;
spec->line[0] = '\0';
+} else if (lineType->id & (LINE_ELIF | LINE_ELIFARCH | LINE_ELIFOS)) {
It is changed in the new version.
--
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/710#discussion_r286490094___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Implement %elif (#710)
pmatilai commented on this pull request.
> rl->lastConditional = lineType;
spec->readStack = rl;
spec->line[0] = '\0';
+} else if (lineType->id & (LINE_ELIF | LINE_ELIFARCH | LINE_ELIFOS)) {
The various ELIF's are another candidate for a bit mask, this set is repeated
at least twice in the patch (used earlier in the "dont expand in false branch"
check). In general, when addressing something, it pays to look for similar
cases/opportunities/bugs, they're quite often there.
--
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/710#pullrequestreview-240605405___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Implement %elif (#710)
Panu's comments are incorporated. -- 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/710#issuecomment-494773665___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Implement %elif (#710)
pmatilai commented on this pull request.
> /* Don't expand macros (eg. %define) in false branch of %if clause */
-if (!spec->readStack->reading)
+} else if (!spec->readStack->reading)
return 0;
If one branch of if uses { } blocks, all the branches should have them.
--
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/710#pullrequestreview-239978836___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Implement %elif (#710)
pmatilai commented on this pull request.
> @@ -501,17 +506,24 @@ int readLine(rpmSpec spec, int strip)
goto retry;
}
-after_classification:
-if (match != -1) {
+if (lineType->id & (LINE_IF | LINE_IFARCH | LINE_IFNARCH | LINE_IFOS |
+ LINE_IFNOS)) {
This calls for a bitmask, something like LINE_IFANY, which can be used to trap
all the types with one symbol. Makes the line shorter and more readable. (fwiw
no I'm not reviewing, just taking a look)
--
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/710#pullrequestreview-239978224___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
