Re: Change packaging preferences? Pull request threshold?

Mmm'kay.  Would it be reasonable for me to open a PR and start populating
it with commits, so y'all can _see_ where I'm going / headed ?  The PR
would stay open until I'm done or someone calls 'time' ...  That way it
would also often/always be in a state that y'all could capture and merge
work-done-so-far preparing for next release....


On Mon, Aug 25, 2014 at 1:43 PM, Frank Trampe <[email protected]>
wrote:

> If your changes all relate to Python, I think that it's fine to have one
> pull request. I wouldn't expect to have any particularly icky merge
> conflicts, and interleaved commits have seldom caused problems for us.
>
>
>
>
> On Mon, Aug 25, 2014 at 1:05 PM, Thomas Shinnick <[email protected]>
> wrote:
>
>> I'm looking at doing numerous changes to python.c , several of which are
>> concerning Python 3 compatibility, which is broken on around 10 different
>> APIs.  (Mishandling strings using Python 2-style PyBytes_* APIs)
>>
>> But in researching/working on *those*, I find other bits broken.  Such
>> as not handling ref counts correctly upon use of PySequence_GetItem(),
>> which is an unusual API.  Or memory leaks apparently introduced to satisfy
>> compiler warnings!  Or out-of-order Py_INCREF/PY_DECREF's.  Everytime I
>> look closer, something says 'boo'.
>>
>> So one *topic* is Python3 compatibility, but that will be ~10 separate
>> commits, one per API, as I really like to test/verify separately.  (Testing
>> font.printSample() took ~250 lines of Python test code).  I wouldn't want
>> to do one PR per API, but I don't know how long it will take to work
>> through all of these - I wouldn't want to wait weeks to do one PR for all.
>> (When's the next release target?)
>>
>> Another topic might be PySequence_GetItem() ref count issues, but those
>> are scattered *everywhere*.  I don't think it is practical to do just one
>> commit/PR.
>>
>> Then there are the Coverity botherances and -Wall notes and so on.
>>
>>
>> What are your suggested guidelines for when to partition work into
>> separate PRs?
>> How long to wait before submitting "work-so-far" as a new batch of
>> commits in a PR?
>> How much 'change' is too much to review in one PR?
>>
>>
>> I'd really hate to submit one huge PR with 47 commits after more than 3
>> weeks, only to find out I actually got one commit horribly wrong and
>> everything has to be backed out.  So... granularity, topic-relatedness,
>> timeliness, ease-of-review, ....  hmmm?
>>
>>
>> Two examples of totally incidental discoveries:
>>
>>
>> https://github.com/fontforge/fontforge/blob/master/fontforge/python.c#L7820
>>        7820         pt += strlen(pt);
>>     s.b.
>>                     pt += strlen(str);
>>         but I haven't *proved* it yet...
>>
>> and
>>
>> https://github.com/fontforge/fontforge/blob/master/fontforge/python.c#L7786-L7790
>>     What is this code 'for'?  Oops, the 'return' disappeared!
>>         commit  39c2b7e7dc  rrthomas 2014-03-06  "Require Python >= 2.7"
>>
>> https://github.com/fontforge/fontforge/commit/39c2b7e7dc4c427cca01046e2b032acaccbd0a29#diff-bd864a9cbdf26fcfdc3dc905c7445c17
>>
>>
>>
>> ------------------------------------------------------------------------------
>> Slashdot TV.
>> Video for Nerds.  Stuff that matters.
>> http://tv.slashdot.org/
>> _______________________________________________
>> fontforge-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/fontforge-devel
>> http://fontforge.10959.n7.nabble.com/Developer-f3.html
>>
>>
>
>
> ------------------------------------------------------------------------------
> Slashdot TV.
> Video for Nerds.  Stuff that matters.
> http://tv.slashdot.org/
> _______________________________________________
> fontforge-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/fontforge-devel
> http://fontforge.10959.n7.nabble.com/Developer-f3.html
>
>

------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/

_______________________________________________
fontforge-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/fontforge-devel
http://fontforge.10959.n7.nabble.com/Developer-f3.html