Page MenuHomeFreeBSD

[PATCH] net/ntopng: Unbreak
ClosedPublic

Authored by bofh on Dec 24 2014, 11:05 AM.

Details

Reviewers
bapt
marino
Summary
  • Unbreak
  • Change EXTRACT_SUFX to USES
Test Plan

http://pdr.s.ubze.ro/bulk/latest-per-pkg/ntopng/1.2.1_1/

Due to failure of devel/icu can't provide log of 9.3-RELEASE in i386.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

bofh updated this revision to Diff 2844.Dec 24 2014, 11:05 AM
bofh retitled this revision from to [PATCH] net/ntopng: Unbreak.
bofh updated this object.
bofh edited the test plan for this revision. (Show Details)
bofh added reviewers: bapt, marino.
bofh set the repository for this revision to rP FreeBSD ports repository.
marino edited edge metadata.Dec 24 2014, 11:13 AM

The actual patch is fine.
However, we also need to review the commit message.

Is the "summary" your intended commit message?
Personally I always put my commit messages in this format:

line 1: One line synopsis of commit, usually starting with <category>/<portname>:
line 2: blank
line 3 ...: meaningful commit message ("why" is more important than "what", but it's okay to list what changed -- just add the reasons if it's not obvious)
next line: blank
link lines: PR/ submittted by / approved by/ reviewed by block

bofh added a comment.Dec 24 2014, 11:45 AM

Hi,

It will be something like the following:
Line1 : Title
Line2 : Blank
Line3-required : Summary
Next : Blank
Linked Links : PR/Submitted by/Reviewed by etc

Okay, great!

For this review, just prewrite the message and paste it as a comment to this review.
For future reviews, paste the intended commit message as the summary.

Then as a reviewer, I or bapt will be approving both the commit and the message.

Ideally, you'd paste the actual message. However, my last mentee found the review block to be a problem and only provided up to the review block (so a couple of minor issues there slipped though).

I actually pre-write all my messages and commit with "svn commit -F <filename>". Not only does that give an additional chance to edit the message (which I often use) but that might help populate the summary block here.

However, you don't have to prewrite the message. I find it very helpful but it's your choice how you want to work.

bofh added a comment.EditedDec 24 2014, 12:05 PM
net/ntopng: Unbreak

- Unbreak
- Change EXTRACT_SUFX to USES


PR:     196189
Differential Revision:  https://reviews.freebsd.org/D1364
Submitted by:   peter@pean.org
Reviewed by:    marino
Approved by:    marino
Obtained from:
MFC after:
MFH:
Relnotes:
Security:
Sponsored by:
marino accepted this revision.Dec 24 2014, 12:14 PM
marino edited edge metadata.

okay, fortunately this effort was not wasted! (on the email, the PR number is expanded to an URL, but here on the site it looks correct).

  1. empty fields like these: Obtained from:, MFC after:, MFH:, Relnotes:, Security:, Sponsored by:

should be omitted

  1. if the reviewer and approver are the same person, you only need the "approved" line (the review is assumed)
  1. The form would be "approved by: marino (mentor)". You always list the hat of the approver in parentheses
  1. Since either bapt or I may approve it, you can just list "approved by: xxx (mentor)" and change "xxx" after approval

I'll approve the commit contingent on you making those changes to the commit message!

This revision is now accepted and ready to land.Dec 24 2014, 12:14 PM
bofh closed this revision.Dec 28 2014, 8:43 AM