[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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



--- Comment #9 from Evan Klitzke  ---
One more question: is there a way to specify test dependencies separately? The
tests for this package require python-pytest to be installed, but it's not
*actually* a build dependency.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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



--- Comment #8 from Evan Klitzke  ---
Hi Zbigniew and Filip,

I made some changes, and I have a few questions.

Updated spec: https://eklitzke.org/pyflame.spec
Updated SRPM: https://eklitzke.org/pyflame-1.1.1-1.fc25.src.rpm

This addresses all of the feedback except the Py3 stuff, which I have a
question about.

Question: currently Pyflame requires building against either Python 2, or
Python 3. It actually does *not* link against libpython, but it does use the
struct definitions from Python.h to know the struct offsets of various Python
fields, and right now you can only build against one or the other. I think I
might be able to do some stuff upstream to make it possible to build against
both simultaneously, but I'm not sure how much work that will be. What do you
think the best option is here?

I will start reviewing some other Python packages. Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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

Filip Szymański  changed:

   What|Removed |Added

 CC||fszyman...@onet.pl



--- Comment #7 from Filip Szymański  ---
1) Get rid of this: 
%global gittag0 v1.1

2) Change:
Source0:
https://github.com/uber/%{name}/archive/v1.1.tar.gz#/%{name}-v%{version}.tar.gz

to

Source0:
https://github.com/uber/%{name}/archive/v%{version}.tar.gz#/%{name}-v%{version}.tar.gz

This way you are changing the version in one place.
Note: Move Source0 little higher.

3) ./autogen.sh and %configure should be in the %build section.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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

Zbigniew Jędrzejewski-Szmek  changed:

   What|Removed |Added

   Assignee|nob...@fedoraproject.org|zbys...@in.waw.pl



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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

Zbigniew Jędrzejewski-Szmek  changed:

   What|Removed |Added

 CC||zbys...@in.waw.pl
 Blocks|177841 (FE-NEEDSPONSOR) |



--- Comment #6 from Zbigniew Jędrzejewski-Szmek  ---
%description should be augmented: what is good for? Why is better than the
competition? Etc. The description you gave on the mailing list would be a good
start.

New Python packages SHOULD support/use python3. Does pyflame support python3.5?
If yes, you should switch to it. (I'm assuming that one version works equally
well to analyze both python2 and python3 scripts... Actually it'd be good to
mention that in the %description too: "This package supports Python 2 and
Python 3" ?)

You must include the license file in the binary package
[https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text].

You have tests. They SHOULD be run in %check.

README.md should be packaged as %doc.


I can sponsor you into the packagers group. In addition to the package that you
are submitting, I require two-three real reviews of other packages (see
http://fedoraproject.org/PackageReviewStatus/NEW.html). There's plenty of
python packages awaiting review, so you should be able to find something
interesting without any trouble. In your review, please indicate that you are
not a packager yet, hence the review is unofficial. If nobody beats you to it,
you'll be able to finalize those reviews after you become a packager (hopefully
soon ;)). Running fedora-review and carefully looking at the output is a good
start.


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor
-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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



--- Comment #5 from Evan Klitzke  ---
Thanks, I've removed the commit hash.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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



--- Comment #4 from Miro Hrončok  ---
The GitHub is there just in case there's no tagged release. In this case, just
take that commit thing out of there.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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



--- Comment #3 from Evan Klitzke  ---
I was just trying to follow the GitHub packaging guidelines. I actually built
the SRPM using the tag v1.1, not the commit hash. What's the recommendation
here?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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

Miro Hrončok  changed:

   What|Removed |Added

 CC||mhron...@redhat.com



--- Comment #2 from Miro Hrončok  ---
While I'm here, I've noticed the commit hash in the spec. What's the purpose?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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

Miro Hrončok  changed:

   What|Removed |Added

 CC||mhron...@redhat.com
   Assignee|lbal...@redhat.com  |nob...@fedoraproject.org



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org

[Bug 1388294] Review Request: pyflame -- A Ptracing Profiler For Python

[bugzilla]

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

Parag AN(पराग)  changed:

   What|Removed |Added

 CC||package-review@lists.fedora
   ||project.org,
   ||panem...@gmail.com
  Component|python-packaging|Package Review



--- Comment #1 from Parag AN(पराग)  ---
Package reviews should be filed against "Package Review" component.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org