diff mbox series

[FFmpeg-devel,1/2] tools/general_assembly: implement extra GA members

Message ID 20231123091424.25640-1-anton@khirnov.net
State Accepted
Commit 56a8b34b64e47dde75314f1b648f3a6f7a325437
Headers show
Series [FFmpeg-devel,1/2] tools/general_assembly: implement extra GA members | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov Nov. 23, 2023, 9:14 a.m. UTC
---
 tools/general_assembly.pl | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Comments

Anton Khirnov Nov. 26, 2023, 9:18 a.m. UTC | #1
Set pushed.

The general_assembly.pl script should now be usable as the authoritative
source for GA members.
Alexander Strasser Nov. 26, 2023, 3:08 p.m. UTC | #2
On 2023-11-26 10:18 +0100, Anton Khirnov wrote:
> Set pushed.
>
> The general_assembly.pl script should now be usable as the authoritative
> source for GA members.

The patches mostly LGTM.

My Perl knowledge in general is really mostly from 20 years ago.
So if there is any Perl-ish devil in the details I surely have
overlooked it.

Please pardon me if I missed any details on how the program works.
The edge cases are always the tricky stuff...

One thing about this patch and that program in general is a bit
unfortunate: The use of PerlDate is_between.

Here is the doc I found for it:

    $dt->is_between( $lower, $upper )
    Checks whether $dt is strictly between two other DateTime objects.

    "Strictly" means that $dt must be greater than $lower and less than $upper. If it is equal to either object then this method returns false.


AFAIU it affects the script in 2 places:

1. In subroutine get_date_range:
   Here the exact day matching date_ga_rule is treated like
   anything >= date_first_regular

2. In the loop adding the extra member. The member would not be added
   on both, the day they were elected nor the day 2 years after.

Case 1 should be "strictly academical" and thus not really important
because to my knowledge no vote was started on that day.

For case 2 it will be not important on most days, but it would seem
more common and intuitive to use either the closed interval or a
half open interval. Where including the first and the last day or
including the first and excluding the day seem most natural to me.


Best regards,
  Alexander


P.S.
As date calculations always turn out nightmares if you look at them
long enough, it would possible be a good idea to always use UTC and
review how time zones are handled in git CLI.

P.P.S.
For quick reference follow copies for both places referenced above:

Case 1:
    sub get_date_range {
        my ($now) = @_;

        # date on which the GA update rule was established, and the voter list
        # was extraordinarily updated; cf.:
        # * http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2023-October/316054.html
        #   Message-Id <169818211998.11195.16532637803201641594@lain.khirnov.net>
        # * http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2023-November/316618.html
        #   Message-Id <5efcab06-8510-4226-bf18-68820c7c69ba@betaapp.fastmail.com>
        my $date_ga_rule       = DateTime->new(year => 2023, month => 11, day => 06);
        # date when the regular update rule is first applied
        my $date_first_regular = DateTime->new(year => 2024);

        if ($now->is_between($date_ga_rule, $date_first_regular)) {
            return ($date_ga_rule->clone()->set_year($date_ga_rule->year - 3), $date_ga_rule);
        }

        if ($now < $date_ga_rule) {
            print STDERR  "GA before $date_ga_rule is not well-defined, be very careful with the output\n";
        }

        my $cur_year_jan  = $now->clone()->truncate(to => "year");
        my $cur_year_jul  = $cur_year_jan->clone()->set_month(7);
        my $date_until    = $now > $cur_year_jul ? $cur_year_jul : $cur_year_jan;
        my $date_since    = $date_until->clone()->set_year($date_until->year - 3);

        return ($date_since, $date_until);
    }


Case 2:
    foreach my $entry (@extra_members) {
        my $elected = $entry->[2];
        if ($date->is_between($elected, $elected->clone()->set_year($elected->year + 2))) {
            $assembly{$entry->[0]} = $entry->[1];
        }
    }
Zhao Zhili Nov. 27, 2023, 2:37 a.m. UTC | #3
> On Nov 26, 2023, at 23:08, Alexander Strasser <eclipse7@gmx.net> wrote:
> 
> On 2023-11-26 10:18 +0100, Anton Khirnov wrote:
>> Set pushed.
>> 
>> The general_assembly.pl script should now be usable as the authoritative
>> source for GA members.
> 
> The patches mostly LGTM.
> 
> My Perl knowledge in general is really mostly from 20 years ago.
> So if there is any Perl-ish devil in the details I surely have
> overlooked it.
> 
> Please pardon me if I missed any details on how the program works.
> The edge cases are always the tricky stuff...
> 
> One thing about this patch and that program in general is a bit
> unfortunate: The use of PerlDate is_between.
> 
> Here is the doc I found for it:
> 
>    $dt->is_between( $lower, $upper )
>    Checks whether $dt is strictly between two other DateTime objects.
> 
>    "Strictly" means that $dt must be greater than $lower and less than $upper. If it is equal to either object then this method returns false.
> 
> 
> AFAIU it affects the script in 2 places:
> 
> 1. In subroutine get_date_range:
>   Here the exact day matching date_ga_rule is treated like
>   anything >= date_first_regular
> 
> 2. In the loop adding the extra member. The member would not be added
>   on both, the day they were elected nor the day 2 years after.
> 
> Case 1 should be "strictly academical" and thus not really important
> because to my knowledge no vote was started on that day.
> 
> For case 2 it will be not important on most days, but it would seem
> more common and intuitive to use either the closed interval or a
> half open interval. Where including the first and the last day or
> including the first and excluding the day seem most natural to me.
> 
> 
> Best regards,
>  Alexander
> 
> 
> P.S.
> As date calculations always turn out nightmares if you look at them
> long enough, it would possible be a good idea to always use UTC and
> review how time zones are handled in git CLI.
> 
> P.P.S.
> For quick reference follow copies for both places referenced above:
> 
> Case 1:
>    sub get_date_range {
>        my ($now) = @_;
> 
>        # date on which the GA update rule was established, and the voter list
>        # was extraordinarily updated; cf.:
>        # * http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2023-October/316054.html
>        #   Message-Id <169818211998.11195.16532637803201641594@lain.khirnov.net>
>        # * http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2023-November/316618.html
>        #   Message-Id <5efcab06-8510-4226-bf18-68820c7c69ba@betaapp.fastmail.com>
>        my $date_ga_rule       = DateTime->new(year => 2023, month => 11, day => 06);
>        # date when the regular update rule is first applied
>        my $date_first_regular = DateTime->new(year => 2024);
> 
>        if ($now->is_between($date_ga_rule, $date_first_regular)) {
>            return ($date_ga_rule->clone()->set_year($date_ga_rule->year - 3), $date_ga_rule);
>        }
> 
>        if ($now < $date_ga_rule) {
>            print STDERR  "GA before $date_ga_rule is not well-defined, be very careful with the output\n";
>        }
> 
>        my $cur_year_jan  = $now->clone()->truncate(to => "year");
>        my $cur_year_jul  = $cur_year_jan->clone()->set_month(7);
>        my $date_until    = $now > $cur_year_jul ? $cur_year_jul : $cur_year_jan;
>        my $date_since    = $date_until->clone()->set_year($date_until->year - 3);
> 
>        return ($date_since, $date_until);
>    }
> 
> 
> Case 2:
>    foreach my $entry (@extra_members) {
>        my $elected = $entry->[2];
>        if ($date->is_between($elected, $elected->clone()->set_year($elected->year + 2))) {
>            $assembly{$entry->[0]} = $entry->[1];
>        }
>    }

I can only say wow as I know zero about perl. Is it the same reason to write it in perl as
automake so it can be run everywhere?

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Anton Khirnov Nov. 28, 2023, 8:06 a.m. UTC | #4
Quoting Alexander Strasser (2023-11-26 16:08:42)
> On 2023-11-26 10:18 +0100, Anton Khirnov wrote:
> > Set pushed.
> >
> > The general_assembly.pl script should now be usable as the authoritative
> > source for GA members.
> 
> The patches mostly LGTM.
> 
> My Perl knowledge in general is really mostly from 20 years ago.
> So if there is any Perl-ish devil in the details I surely have
> overlooked it.
> 
> Please pardon me if I missed any details on how the program works.
> The edge cases are always the tricky stuff...
> 
> One thing about this patch and that program in general is a bit
> unfortunate: The use of PerlDate is_between.
> 
> Here is the doc I found for it:
> 
>     $dt->is_between( $lower, $upper )
>     Checks whether $dt is strictly between two other DateTime objects.
> 
>     "Strictly" means that $dt must be greater than $lower and less than $upper. If it is equal to either object then this method returns false.
> 
> 
> AFAIU it affects the script in 2 places:
> 
> 1. In subroutine get_date_range:
>    Here the exact day matching date_ga_rule is treated like
>    anything >= date_first_regular
> 
> 2. In the loop adding the extra member. The member would not be added
>    on both, the day they were elected nor the day 2 years after.
> 
> Case 1 should be "strictly academical" and thus not really important
> because to my knowledge no vote was started on that day.
> 
> For case 2 it will be not important on most days, but it would seem
> more common and intuitive to use either the closed interval or a
> half open interval. Where including the first and the last day or
> including the first and excluding the day seem most natural to me.

These objects are not days, they are specific times with nanosecond
resolution. Since I'm not specifying time in the constructor, it
defaults to the first nanosecond-long instant of the day, so none of
these problems really apply.
Michael Niedermayer Dec. 6, 2023, 7:45 p.m. UTC | #5
On Thu, Nov 23, 2023 at 10:14:23AM +0100, Anton Khirnov wrote:
> ---
>  tools/general_assembly.pl | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/general_assembly.pl b/tools/general_assembly.pl
> index 4c3208ccac..3bf65f3405 100755
> --- a/tools/general_assembly.pl
> +++ b/tools/general_assembly.pl
> @@ -13,6 +13,12 @@ use utf8;
>  use DateTime;
>  use DateTime::Format::ISO8601;
>  
> +my @extra_members = (
> +    # entries should be of the format
> +    # [   <name>,   <email>, <date elected> ],
> +    # ['Foo Bar', 'foo@bar', DateTime->new(year => 8613, month => 5, day => 22)],
> +);
> +
>  sub trim { my $s = shift; $s =~ s/^\s+|\s+$//g; return $s };
>  
>  sub print_help {
> @@ -29,7 +35,7 @@ sub print_help {
>  my $print_full = 1;
>  my $print_names = 0;
>  my $print_emails = 0;
> -my $date         = DateTime->now()->iso8601;
> +my $date_str     = DateTime->now()->iso8601;
>  my $help = 0;
>  
>  GetOptions(
> @@ -37,7 +43,7 @@ GetOptions(
>      "names" => \$print_names,
>      "emails" => \$print_emails,
>      "help" => \$help,
> -    "date=s" => \$date,
> +    "date=s" => \$date_str,
>      "h" => \$help,
>  );
>  
> @@ -76,7 +82,8 @@ sub get_date_range {
>      return ($date_since, $date_until);
>  }
>  
> -my ($since, $until) = get_date_range(DateTime::Format::ISO8601->parse_datetime($date));
> +my $date = DateTime::Format::ISO8601->parse_datetime($date_str);
> +my ($since, $until) = get_date_range($date);
>  
>  my @shortlog = split /\n/, decode('UTF-8',
>      `git log --pretty=format:"%aN <%aE>" --since="$since" --until="$until" | sort | uniq -c | sort -r`,
> @@ -108,6 +115,13 @@ foreach my $line (@shortlog) {
>      $assembly{$name} = $email;
>  }
>  
> +foreach my $entry (@extra_members) {
> +    my $elected = $entry->[2];
> +    if ($date->is_between($elected, $elected->clone()->set_year($elected->year + 2))) {

tools/general_assembly.pl
Can't locate object method "is_between" via package "DateTime" at tools/general_assembly.pl line 75.

is_between seems to have been added in 1.52   2020-02-29
So this is unavailable in several distros, it works with cpan though
but this shoould be docuemnted if it cannot be avoided

thx

[...]
diff mbox series

Patch

diff --git a/tools/general_assembly.pl b/tools/general_assembly.pl
index 4c3208ccac..3bf65f3405 100755
--- a/tools/general_assembly.pl
+++ b/tools/general_assembly.pl
@@ -13,6 +13,12 @@  use utf8;
 use DateTime;
 use DateTime::Format::ISO8601;
 
+my @extra_members = (
+    # entries should be of the format
+    # [   <name>,   <email>, <date elected> ],
+    # ['Foo Bar', 'foo@bar', DateTime->new(year => 8613, month => 5, day => 22)],
+);
+
 sub trim { my $s = shift; $s =~ s/^\s+|\s+$//g; return $s };
 
 sub print_help {
@@ -29,7 +35,7 @@  sub print_help {
 my $print_full = 1;
 my $print_names = 0;
 my $print_emails = 0;
-my $date         = DateTime->now()->iso8601;
+my $date_str     = DateTime->now()->iso8601;
 my $help = 0;
 
 GetOptions(
@@ -37,7 +43,7 @@  GetOptions(
     "names" => \$print_names,
     "emails" => \$print_emails,
     "help" => \$help,
-    "date=s" => \$date,
+    "date=s" => \$date_str,
     "h" => \$help,
 );
 
@@ -76,7 +82,8 @@  sub get_date_range {
     return ($date_since, $date_until);
 }
 
-my ($since, $until) = get_date_range(DateTime::Format::ISO8601->parse_datetime($date));
+my $date = DateTime::Format::ISO8601->parse_datetime($date_str);
+my ($since, $until) = get_date_range($date);
 
 my @shortlog = split /\n/, decode('UTF-8',
     `git log --pretty=format:"%aN <%aE>" --since="$since" --until="$until" | sort | uniq -c | sort -r`,
@@ -108,6 +115,13 @@  foreach my $line (@shortlog) {
     $assembly{$name} = $email;
 }
 
+foreach my $entry (@extra_members) {
+    my $elected = $entry->[2];
+    if ($date->is_between($elected, $elected->clone()->set_year($elected->year + 2))) {
+        $assembly{$entry->[0]} = $entry->[1];
+    }
+}
+
 # generate the output string
 my @out_lines;
 foreach my $name (sort keys %assembly) {