[Rpm-maint] [rpm-software-management/rpm] Support new %[ ] expression expansion syntax (#846)

Panu Matilainen notifications at github.com
Fri Sep 20 10:49:22 UTC 2019


Yeah, looks that little bit nicer that way, thanks. I don't particularly love camelCase but when everything in the surroundings uses it... 

There's an unrelated indentation change for the division-by-zero case, and trailing whitespace after the doExpressionExpansion() function. I could live with both, just noting for the record. I do regret not re-indenting the whole expression.c to generic rpm style while moving to rpmio/ when the whole thing had been essentially untouched for a decade, doing so now would just mess the recent history unnecessarily.

That "contains non-digits" error message baffled me initially, had to fool around with it before getting it. I wonder if something to the tune of "expected an integer" would be more to the point, or maybe something like the bare word error. The expansion output is indeed necessary to make sense out of it regardless of the actual message.

Other than those minor nits / pondering, I've no issues with this. 

Oh and sorry about the conflict (from the double-free thing), care to do one more rebase to resolve that? I promise not to touch expression.c and macro.c again before this is 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/846#issuecomment-533505345
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20190920/1105b8c5/attachment.html>


More information about the Rpm-maint mailing list