119502 - reject patches based on what diff format they are in

Status: RESOLVED WONTFIX

(Bugzilla :: Administration, task, P4)

Description

[John Vandenberg]

Context diffs are much harder to review, and mostly it is simpler to just
request the developer attach the patch in a different format.  Bugzilla can
enforce this process by rejecting the patch, giving the developer a nice message
describing how they can create a Unified patch.

Comment 1

[Brian 'netdragon' Bober]

This would be good for b.m.o, but would be useless for projects that use other
kinds of version management systems. I think it might be better to be able to
have a patch filter that can be written in perl and included in the bugzilla
dir. It could be made so this filter is easy to add. The filter would scan the
patch (In this case, make sure its the right type).

Comment 2

[John Vandenberg]

I have added the filter to my local copy of Patch.pm (see bug 115910), and added
a param to turn it off for sites which there is no parser for.  It should be
easy enough to add other formats as required for other sites.

Comment 3

[John Vandenberg]

Attachment: patch to use Patch.pm

Comment 4

[John Vandenberg]

Attachment: Patch.pm

small Patch.pm with only sub Format

Comment 5

[John Vandenberg]

New Patch.pm on bug 115910.

Comment 6

[John Vandenberg]

This should be able to go into 2.18 as that is the release bug 115910 is
targetted for.

Comment 7

[John Keiser (jkeiser)]

I am not sure if this is necessary anymore, if bug 174942 goes in.  With Patch
Viewer, we can transform a normal diff into a unified diff so that people who
want to view it in context or apply it will be able to do so easily.  It cannot,
however, add context for patches if a Bugzilla does not have a cvs repository,
so if that is the main reason these patches are easier to review, this will
still be useful.

For b.m.o, I think bug 174942 will be enough.

Comment 8

[John Keiser (jkeiser)]

Patch Viewer is in the tree, and allows the user to turn any diff format (at
least any of the ones I found) into raw unified.  This is technically orthogonal
to the problem in this bug, however.  A few small changes could be made to
PatchIterator to make it report the type of diff it found, so that we don't have
two patch parsers, but in the end that particular point of code sharing probably
doesn't matter much.

Comment 9

[Joel Peshkin]

Unloved bugs targetted for 2.18 but untouched since 9-15-2003 are being
retargeted to 2.20
If you plan to act on one immediately, go ahead and pull it back to 2.18.

Comment 10

[Dave Miller [:justdave]]

Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.

Comment 11

[Kevin Benton]

There are times when a unified diff is not possible.  Rejecting a patch is terminal.  Warning the user and offering them an opportunity to confirm yes, really use a context diff is much more user-friendly.

Comment 12

[Frédéric Buclin]

Rejecting patches means Bugzilla should understand many different formats (git, CVS, bzr, ...) and make sure the format is incorrect (or rather: unwanted). That's not something I want upstream as it adds complexity for mostly no benefit.