Conversation
| // We needs to replace all public constants to static readonly fields to allow | ||
| // binary substitution of different Python.Runtime.dll builds in a target application. | ||
|
|
||
| public static string pyversion => _pyversion; |
There was a problem hiding this comment.
@filmor I think a unit test depends on these public properties
There was a problem hiding this comment.
Which one? I was used in a few places, but only to check for Python 2 vs 3.
There was a problem hiding this comment.
@filmor you already removed them actually from TestPythonEngineProperties.cs
Seems like this might require an entry in the changelog since its a breaking change technically
There was a problem hiding this comment.
Yeah, I could do that, but pyversion etc. where never really consumables in the first place. I'd like to move the whole Runtime to internal as a single Changelog entry, no need to do this for every single component.
There was a problem hiding this comment.
@filmor moving Runtime to internal deserves its own tracking issue.
|
@filmor have you tried just changing the runtime and leaving the tests as-is for a first pass? I looked through the runtime and didn't see a mistake so I think I would start by ruling out problems with the test updating (which can easily be done in a followon branch) |
|
Not needed. The failures before were due to one place where I forgot to replace |
Codecov Report
@@ Coverage Diff @@
## master #1158 +/- ##
==========================================
- Coverage 86.53% 86.25% -0.28%
==========================================
Files 1 1
Lines 297 291 -6
==========================================
- Hits 257 251 -6
Misses 40 40
Continue to review full report at Codecov.
|
lostmsu
left a comment
There was a problem hiding this comment.
OK by me after a couple minor fixes
| // We needs to replace all public constants to static readonly fields to allow | ||
| // binary substitution of different Python.Runtime.dll builds in a target application. | ||
|
|
||
| public static string pyversion => _pyversion; |
There was a problem hiding this comment.
@filmor moving Runtime to internal deserves its own tracking issue.
|
Also, if you don't increase the coverage back to the target, or lower the target a bit, it is going to complain on every subsequent PR. |
|
The coverage is always against master, it will not repeatedly complain. |
Drop Python 2 support
What does this implement/fix? Explain your changes.
Drops support for and all references to Python 2.
Does this close any currently open issues?
Can't find it right now, but I think we had an issue discussing this.
Any other comments?
-/-
Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG