[Bug 691027] Review Request: n2n - A layer-two peer-to-peer virtual private network

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=691027

Michel Alexandre Salim <michel+fdr at sylvestre.me> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |needinfo?(hjia at redhat.com)

--- Comment #11 from Michel Alexandre Salim <michel+fdr at sylvestre.me> 2011-04-01 12:00:43 EDT ---
Hi Hushan,

Looking at the package now. Some questions; I'll continue reviewing afterwards.
Thanks for doing the informal reviews -- it helps packagers when they do the
full review. That package looks neat, so I'll actually review it and check on
your informal review at the same time.

I'll continue this review after the issues below are addressed. Thanks!

=== Source ===

- any reason you're tracking SVN trunk? The n2n-2.0.1 tag might be a better
bet:
  https://svn.ntop.org/svn/ntop/tags/n2n-2.0.1/

- rather than checking out using Subversion and then using a patched version of
  upstream's script for packaging, I'd suggest checking out using git's svn
  adapter. I find `svn export` frustrating as it exports a directory, and thus
  the file ordering is unspecified if you then directly use tar. The files are
  added in the order that the filesystem provides the directory listing, and
even
  for the same SVN revision this is not the same!

  (note that even with trunk, the last revision for n2n is 4444, not 4541 --
the
   later revisions do not touch n2n at all)

    git svn clone --no-minimize-url
https://svn.ntop.org/svn/ntop/tags/n2n-2.0.1
    (cd n2n-2.0.1 && git archive HEAD --format=tar --prefix=n2n-2.0.1/) | bzip2
- > n2n-2.0.1.tar.bz2


  In an ideal world, upstream has this layout:

    /svn/n2n/{trunk,branches,tags}

  that way one can just do this:

    git svn clone --no-minimize-url -s https://svn.ntop.org/svn/n2n

  and then keep track of upstream developments by pulling in later SVN
  revisions:
    git svn rebase

  rather than checking out a tag whenever it is created. Their current SVN
  layout is rather messy.

  Could you perhaps ask them to also release n2n tarballs? This does not have
  to delay this review, but would make maintaining this package easier.

=== Optimization flags ===
Fabian raised this issue, which you have not addressed. Since this package does
not use the standard %configure, you'd have to override CFLAGS manually when
building. e.g.

make %{?_smp_mflags} CFLAGS=%{optflags}

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.