Fix and enforce BIP 2 & 3 field ordering#1893
Closed
nothingmuch wants to merge 3 commits into
Closed
Conversation
nothingmuch
force-pushed
the
field-ordering
branch
from
6707c35 to
6aa4213
Compare
-BEGIN VERIFY SCRIPT-
set -e
perl <<'-END PERL-'
use strict;
use warnings;
my $topbip = 9999;
my @FieldOrder = qw(
BIP
Layer
Title
Author
Authors
Editor
Deputies
Discussions-To
Comments-Summary
Comments-URI
Status
Type
Created
License
License-Code
Discussion
Post-History
Version
Requires
Replaces
Proposed-Replacement
Superseded-By
);
my $bipnum = 0;
while (++$bipnum <= $topbip) {
my $fn = sprintf "bip-%04d.mediawiki", $bipnum;
my $is_markdown = 0;
if (!-e $fn) {
$fn = sprintf "bip-%04d.md", $bipnum;
$is_markdown = 1;
}
-e $fn || next;
open my $F, "<", $fn or die "$!";
my (@before, %preamble, @after);
if ($is_markdown) {
while (<$F>) {
push @before, $_;
last if m[^(?:\xef\xbb\xbf)?```$]
}
die "No ``` in $fn" if eof $F;
} else {
while (<$F>) {
push @before, $_;
last if m[^(?:\xef\xbb\xbf)?<pre>$];
}
die "No <pre> in $fn" if eof $F;
}
my %found;
my ($title, $author, $status, $type, $layer);
my ($field, $val, @field_order);
while (<$F>) {
push @after, $_ and last if ($is_markdown && m[^```$]);
push @after, $_ and last if (!$is_markdown && m[^</pre>$]);
if (m[^ ([\w-]+)\: (.*\S)$]) {
$field = $1;
$val = $2;
} elsif (m[^ ( +)(.*\S)$]) {
$val = $2;
} else {
die "Bad line in $fn preamble";
}
push @{$preamble{$field} ||= []}, $_;
}
push @after, <$F>;
close $F or die $!;
open my $W, ">", "$fn" or die "$!";
print $W @before;
print $W map { @$_ } grep { defined } delete @Preamble{@FieldOrder};
die "Unknown fields: @{[ keys %preamble ]}" if %preamble;
print $W @after;
close $W or die $!;
}
-END PERL-
-END VERIFY SCRIPT-
nothingmuch
force-pushed
the
field-ordering
branch
from
6aa4213 to
69d5303
Compare
nothingmuch
changed the title
The specified field order is consistent with both BIPs 2 and 3. The ordering of fields which are only present in one or the other is ambiguous, e.g. as in `Proposed-Replacement` and `Superseded-By` but only one of these applies to a given BIP. The `Editor` field is spurious, only being used in BIP 69, and appears after Author.
nothingmuch
force-pushed
the
field-ordering
branch
from
69d5303 to
2bcf420
Compare
Contributor
Author
murchandamus
added
the
Process
jonatack
reviewed
Jul 17, 2025
| die "Unknown field $field in $fn"; | ||
| } | ||
| ++$found{$field}; | ||
| push @field_order, $field if $field_order[-1] ne $field; |
Member
There was a problem hiding this comment.
Running ./scripts/buildtable.pl on this branch, I am seeing the following messages on each iteration that were not present on master:
Use of uninitialized value in string ne at ./scripts/buildtable.pl line 209, <$F> line 2.
Member
There was a problem hiding this comment.
$ ./scripts/buildtable.pl
Use of uninitialized value in string ne at ./scripts/buildtable.pl line 209, <$F> line 2.
|- style="background-color: #ffcfcf"
| [[bip-0001.mediawiki|1]]
|
| BIP Purpose and Guidelines
| Amir Taaki
| Process
| Replaced
Use of uninitialized value in string ne at ./scripts/buildtable.pl line 209, <$F> line 2.
|- style="background-color: #cfffcf"
| [[bip-0002.mediawiki|2]]
|
| BIP process, revised
| Luke Dashjr
| Process
| Active
Use of uninitialized value in string ne at ./scripts/buildtable.pl line 209, <$F> line 2.
|- style="background-color: #ffffcf"
| [[bip-0003.md|3]]
|
| Updated BIP Process
| Murch
| Process
| Proposed
Use of uninitialized value in string ne at ./scripts/buildtable.pl line 209, <$F> line 2.
.../...
Contributor
Author
There was a problem hiding this comment.
sorry, that was sloppy... i pushed a fixup commit, should i squash it already or would reviewing that on its own be easier?
Member
There was a problem hiding this comment.
Thanks -- rebased the fixup and merged the pull manually.
jonatack
added
the
PR Author action required
jonatack
removed
the
PR Author action required
Member
|
ACK (thanks!), ran and verified the scripted diff and build table change locally. This has been merged with 0f8a226. |
Member
|
Thanks @nothingmuch and @jonatack for landing this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The first commit is a scripted diff that corrects field ordering in a number of BIPs which are not consistent with the specified in BIP 2.
The second commit enforces that fields are ordered consistently with both BIP 2 and BIP 3. It does not strictly depend on #1820.