Re: [Jython-dev] Pull requests #36 and #38

[Darjus L. <da...@gm...>]

Hey Stefan,

That's great diligence. The changes look safe to me.

Cheers,
Darjus

On Sun, May 1, 2016 at 12:51 PM Stefan Richthofer <Ste...@gm...>
wrote:

> Jim, thanks for your kind reply.
>
> >we generally follow a commit-then-review (CTR) process
> Alright. I was aware of this workflow implicitly so far. For #36 and #38 I
> was just looking for an explicit 'okay' even for save stuff, because it's
> in the hot shortly-before-release-phase (and it had been postulated that
> new features should go into 2.7.2, which was stated assuming less delay for
> 2.7.1 I suppose).
>
>
> *Gesendet:* Sonntag, 01. Mai 2016 um 03:40 Uhr
> *Von:* "Jim Baker" <jim...@py...>
> *An:* "Stefan Richthofer" <Ste...@gm...>
> *Cc:* "Jython Developers" <jyt...@li...>
> *Betreff:* Re: [Jython-dev] Pull requests #36 and #38
> On Sat, Apr 30, 2016 at 5:34 AM, Stefan Richthofer <
> Ste...@gm...> wrote:
>>
>> Hello everybody,
>>
>> given that Jython 2.7.1 was delayed so long (actually we're about to
>> reach the planned release date for 2.7.2)
>
>
> Yes, I'm really well aware of how much we have slipped. On the other hand,
> this past week I had a chance to see a demo of some soon to be production
> code using our latest work, running on Twisted with SSL connections and
> against Java and Clojure libraries, all with quite reasonable performance
> due to no GIL.
>
> So at this time, I'm hoping that we can complete by PyCon; or at least the
> PyCon sprints for those attending. See bit.ly/jython-triage-2_7_1 for
> outstanding bugs. Bugs of urgent/immediate severity block the release
> candidate; but we should include with respect to the high bugs as well, all
> of which have outstanding fixes to be applied.
>
> The real challenge is the immediate bug ("the publication bug" for
> class/type mappings in http://bugs.jython.org/issue2487); it's something
> Darjus Loktevic and I have put in some time into, but so far, not resolved.
> But the final exam in the course I'm teaching is this Monday; and I just
> got back from the OpenStack Summit where I was kicking off a new project.
> So I personally have some time to squash that bug coming up real soon now.
>
>
>> I'd like to merge the pull requests
>>
>> https://github.com/jythontools/jython/pull/36
>>
>> https://github.com/jythontools/jython/pull/38
>>
>> before the release. I think there is no risk to introduce new issues with
>> these changes, because they really do simple stuff or just add a function.
>
>
> +1. For committers, we generally follow a commit-then-review (CTR)
> process. The other committers haven't had a chance to review these PRs in
> advance - certainly not me -  but in general - simple stuff/adds a function
> has worked safely for us with CTR. Basically any committer should know when
> to reach out to another one for review of the tricky bits - that's why we
> were so designated :)
>
> See my separate email sent on this same thread on how we actually merge
> such PRs. (tl;dr it's manual.)
>
>
>>
>> Regarding #36:
>> Given that there was no uname in Jython before there cannot be (sane*)
>> code that could be affected even if my implementation was faulty.
>>
>> * code might be conditioned on uname not existing, but I regard it a
>> no-goal to preserve bug-compatibility to old versions
>>
>>
>> Regarding #38:
>> Changes only concern if-branches that don't apply to Jython anyway.
>> Actually these branches could have been removed completely (stuff that is
>> conditioned onto e.g. os.name == 'nt'). I did not remove them because
>> Jython code might want to hack around with them, e.g. by actively setting
>> os.name to some value != 'java' (code I doubt to exist anyway). However
>> for JyNI I need these branches additionally secured by a check for !=
>> 'java' for reasons that go too far to explain here (I can give details on
>> the use case on demand).
>> So semantically these checks don't change anything for Jython at all;
>> they even reduce the number of failing checks a bit, as os.name !=
>> 'java' usually wraps an entire block of several checks for various
>> platforms not applying to Jython-case.
>>
>> If there are no concerns I would check these into Jython someday next
>> week. Presumably these commits would allow me to drop the JyNI-specific
>> ctypes python code and use the CPython-bundled ctypes entirely. #36 is also
>> required for JyOpenGL.
>>
>> Regards
>>
>> Stefan
>>
>>
>> ------------------------------------------------------------------------------
>> Find and fix application performance issues faster with Applications
>> Manager
>> Applications Manager provides deep performance insights into multiple
>> tiers of
>> your business applications. It resolves application problems quickly and
>> reduces your MTTR. Get your free trial!
>> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
>> _______________________________________________
>> Jython-dev mailing list
>> Jyt...@li...
>> https://lists.sourceforge.net/lists/listinfo/jython-dev
>
> ------------------------------------------------------------------------------
> Find and fix application performance issues faster with Applications
> Manager Applications Manager provides deep performance insights into
> multiple tiers of your business applications. It resolves application
> problems quickly and reduces your MTTR. Get your free trial!
> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z_______________________________________________
> Jython-dev mailing list Jyt...@li...
> https://lists.sourceforge.net/lists/listinfo/jython-dev
>
> ------------------------------------------------------------------------------
> Find and fix application performance issues faster with Applications
> Manager
> Applications Manager provides deep performance insights into multiple
> tiers of
> your business applications. It resolves application problems quickly and
> reduces your MTTR. Get your free trial!
> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
> _______________________________________________
> Jython-dev mailing list
> Jyt...@li...
> https://lists.sourceforge.net/lists/listinfo/jython-dev
>