Skip to content

Conversation

Ekopalypse
Copy link
Contributor

Make tests run with scintilla 4.2.0

@Ekopalypse
Copy link
Contributor Author

Is there a problem with this one?
The already merged commit, the one for the samples, shouldn't appear here, shouldn't it?
Should I have created two branches derived from the same master branch?

@@ -801,7 +804,7 @@ def test_scintillawrapper_colour_int_void_in_callback(self):
self.assertEqual(self.callbackCalled, True)

def test_scintillawrapper_bool_int_void(self):
original = editor.getMarginSensitiveN(1)
_ = editor.getMarginSensitiveN(1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you change this? Is there any need for it or just because it is an unused value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need, depends, this and the other change is basically to satisfy the python linter.

@@ -2,7 +2,10 @@
import unittest
import time

from Npp import *
from Npp import (
Copy link
Collaborator

@chcg chcg Oct 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are the imports in braces?
What is the benefit to import just the necessary module parts for the tests?

Copy link
Contributor Author

@Ekopalypse Ekopalypse Oct 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, both are Linter issues.

@chcg
Copy link
Collaborator

chcg commented Oct 28, 2019

@Ekopalypse Let's see how smart the git merge is.

Just didn't merge because of some questions regarding the changes, but forgot to click submit for my review comments.

@chcg chcg merged commit e3931b6 into bruderstein:master Oct 28, 2019
@Ekopalypse Ekopalypse deleted the AdaptTestsToSci420 branch October 28, 2019 18:05
@chcg chcg added this to the v1.5 milestone Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants