Re: Change packaging preferences? Pull request threshold?
Frank Trampe <[email protected]>
| Newsgroups | gmane.comp.fonts.fontforge.devel |
|---|---|
| Message-ID | <CANkSbhp_4GnBO7Ysfd0PshqovUVZh9Y4VWVMBe5qieRQ0_ubhw@mail.gmail.com> |
That works. Don't worry about the release timing. I am going to guess that you'll finish this effort long before we're ready for the big release. Plus, with things moving as quickly as they are, we'll probably have fairly frequent minor releases through the end of the year. On Mon, Aug 25, 2014 at 2:07 PM, Thomas Shinnick <[email protected]> wrote: > 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 > > ------------------------------------------------------------------------------ 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