master: replace most str occurences by string_types #3139
Conversation
When configuring some services in master.cfg with:
from __future__ import unicode_literals
from buildbot.plugins.db import steps
b = steps.ShellCommand(
name='do stuff',
command=['stuff'],
)
there are errors with python 2:
BuildStep name must be a string: u'do stuff'
BuildStep is only an example, there are other constructors that do not
accept unicode strings.
Since almost all internal API work on unicode strings, replace most
checks that a variable is a string by testing if it is an instance of
string_types (from future.utils).
Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
Codecov Report
@@ Coverage Diff @@
## master #3139 +/- ##
==========================================
+ Coverage 88.32% 88.32% +<.01%
==========================================
Files 314 314
Lines 32995 33007 +12
==========================================
+ Hits 29142 29154 +12
Misses 3853 3853
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
On the principle I of course agree with this change.
However this is not really tested.
We should have tests ensuring the step works with both types.
Another option is to enforce unicode everywhere.
from buildbot.util import bytes2unicode
if isintance(foo, string_types)
foo = bytes2unicode(foo)
else:
config.error("foo must be string or unicode")That way, I will be more confident that the unit tests will catch incompatibilities
|
Ok I understand. I'll have a look at maybe enhancing the tests and force convert to unicode. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
closing due to our stalled pull request policy |
|
Removing labels as we no longer need this in master. |
When configuring some services in
master.cfgwith:there are errors with python 2:
BuildStepis only an example, there are other constructors that do not accept unicode strings.Since almost all internal API work on unicode strings, replace most checks that a variable is a string by testing if it is an instance of
string_types(fromfuture.utils).Contributor Checklist: