Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2617)

Issue 555260044: Fix convert-ly with Python 3 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 2 months ago by hahnjo
Modified:
4 years, 2 months ago
Reviewers:
dak
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Fix convert-ly with Python 3 The error was "'str' object has no attribute 'decode'", so it already is decoded. The encoding seems to also be correct, as filenames with Unicode characters work just fine. It's worrying that none of the usual testing commands seem to invoke the script with a real input file, only the output of --help is used. --- If possible I'd like to push this as soon as possible. The normal process fails on convert-ly anyhow...

Patch Set 1 #

Total comments: 2

Patch Set 2 : Drop line #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -1 line) Patch
M scripts/convert-ly.py View 1 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 5
dak
Foreseeable consequences for Python 2.7?
4 years, 2 months ago (2020-02-04 19:32:10 UTC) #1
hahnjo
On 2020/02/04 19:32:10, dak wrote: > Foreseeable consequences for Python 2.7? None, because the minimum ...
4 years, 2 months ago (2020-02-04 19:40:41 UTC) #2
dak
https://codereview.appspot.com/555260044/diff/567150044/scripts/convert-ly.py File scripts/convert-ly.py (right): https://codereview.appspot.com/555260044/diff/567150044/scripts/convert-ly.py#newcode361 scripts/convert-ly.py:361: f = f That line looks spurious. Any reason ...
4 years, 2 months ago (2020-02-04 19:45:34 UTC) #3
hahnjo
Drop line
4 years, 2 months ago (2020-02-04 19:49:51 UTC) #4
hahnjo
4 years, 2 months ago (2020-02-04 19:50:23 UTC) #5
https://codereview.appspot.com/555260044/diff/567150044/scripts/convert-ly.py
File scripts/convert-ly.py (right):

https://codereview.appspot.com/555260044/diff/567150044/scripts/convert-ly.py...
scripts/convert-ly.py:361: f = f
On 2020/02/04 19:45:33, dak wrote:
> That line looks spurious.  Any reason for keeping it in?

Hm, no :D
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b