[Rpm-maint] [PATCH] Extend %changelog to support full timestamps

Panu Matilainen pmatilai at laiskiainen.org
Thu Oct 6 15:49:38 UTC 2016


On 10/06/2016 05:36 PM, Florian Festi wrote:
> On 10/06/2016 02:33 PM, Florian Weimer wrote:
>> On 10/06/2016 09:10 AM, Pavlina Varekova wrote:
>>> Extend %changelog to support full timestamps
>>>
>>> The newly accepted date format is
>>>
>>> Mon Jan 6 09:02:22 CEST 2016
>>>
>>> (like output of "date" command). Original format "Mon Jun 6 2016" is
>>> still
>>> supported.
>>
>> I think it would be better to use ISO 8601 format (perhaps with space
>> instead of 'T' as the date/time separator).  The week day information is
>> redundant and only causes consistency issues.
>
> I am really not a fan of the format but it is what's already implemented
> in rpm - especially on output with the :date and :day formats (which
> basically do date %a %b %d %Y and date %c). So this is the right way to
> go. I agree that the days of week are somehow redundant and an
> annoyance. But they ca be made optional later in another patch.
>
> Wrt the actual code: It's just a minor nit pick, but there are several
> lines duplicated in the two if / else blocks. Parsing the year and
> generating the final result can probably be done outside of the if else
> and those saving a few lines.

Yup, copy-paste code is bad even in small quantities, it'll bite back 
one day.

Another minor nit:

/*
  * This is the rpm multi-line comment style, for better
  * or worse.
  */

Bonus points available by making the code more readable - cramming 
multiple statements and if-actions etc into a single line is not a good 
practise. The existing code doing something stupid is not a good reason 
to duplicate it in new code  :)

A whole other perspective on the parsing: is there an actual reason 
strptime() cannot be used? It's not like I've tried, but I'd think you 
could just call it with couple of different formats, or maybe even make 
the "extended" format configurable via a macro.

	- Panu -


More information about the Rpm-maint mailing list