Convert all script metadata to text #23644
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wpt-pr-bot
added
infra
manifest
serve
wptrunner
wpt-pr-bot
requested review from
Hexcles,
KyleJu,
LukeZielinski and
stephenmcgruer
Hexcles
reviewed
May 15, 2020
Hexcles
reviewed
May 15, 2020
jgraham
force-pushed
the
script_metadata_text
branch
from
f9b2d22
to
6edd225
Compare
jgraham
force-pushed
the
script_metadata_text
branch
from
6edd225
to
61eada9
Compare
jgraham
force-pushed
the
script_metadata_text
branch
from
61eada9
to
c16e45f
Compare
foolip
reviewed
May 18, 2020
foolip
reviewed
May 18, 2020
tools/manifest/sourcefile.py
Outdated
| assert isinstance(value, text_type), value | ||
| except Exception: | ||
| import pdb | ||
| pdb.post_mortem() |
There was a problem hiding this comment.
Did you add this to debug something? Most other pdb.post_mortem() go together with a traceback.print_exc(), and many are also conditional. So it's hard to tell if this is leftover debugging or useful to leave here.
There was a problem hiding this comment.
Oops. Pure debugging leftover.
foolip
approved these changes
May 18, 2020
Otherwise we were getting a different type when loading the manifest compared to when reading data from the initial file. This caused problems in Python 3.
jgraham
force-pushed
the
script_metadata_text
branch
from
dfd5cb7
to
6ddad08
Compare
stephenmcgruer
added a commit
that referenced
this pull request
May 19, 2020
Reusing variables can cause a mypy type error (python/mypy#1174). This previously went unnoticed because the previous and current use of `key` was `str`, but in #23644 the previous use was retyped to `Text` and as such caused a unicode-vs-str type error in mypy.
Hexcles
reviewed
May 19, 2020
| @@ -864,11 +864,11 @@ def manifest_items(self): | |||
| )] | |||
|
|
|||
| elif self.name_is_multi_global: | |||
| globals = b"" | |||
| globals = u"" | |||
There was a problem hiding this comment.
Nit: it's a bit inconsistent that some text strings are prefixed with u while the others are not. Shall we apply the future lint for string literals to tools/ as well? @jgraham
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
May 27, 2020
…, a=testonly Automatic update from web-platform-tests Rename variable to avoid mypy type error (#23669) Reusing variables can cause a mypy type error (python/mypy#1174). This previously went unnoticed because the previous and current use of `key` was `str`, but in web-platform-tests/wpt#23644 the previous use was retyped to `Text` and as such caused a unicode-vs-str type error in mypy. -- wpt-commits: 7b9a66f8a9cf68bbe220473f9eff0e652998c072 wpt-pr: 23669
xeonchen
pushed a commit
to xeonchen/gecko
that referenced
this pull request
May 27, 2020
…, a=testonly Automatic update from web-platform-tests Rename variable to avoid mypy type error (#23669) Reusing variables can cause a mypy type error (python/mypy#1174). This previously went unnoticed because the previous and current use of `key` was `str`, but in web-platform-tests/wpt#23644 the previous use was retyped to `Text` and as such caused a unicode-vs-str type error in mypy. -- wpt-commits: 7b9a66f8a9cf68bbe220473f9eff0e652998c072 wpt-pr: 23669
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Otherwise we were getting a different type when loading the manifest
compared to when reading data from the initial file. This caused problems
in Python 3.