Skip to content

When reading config files, require them to be encoded with UTF-8.#1735

Merged
jaraco merged 6 commits into
masterfrom
bugfix/1702-utf8-config
Apr 5, 2019
Merged

When reading config files, require them to be encoded with UTF-8.#1735
jaraco merged 6 commits into
masterfrom
bugfix/1702-utf8-config

Conversation

@jaraco

@jaraco jaraco commented Apr 5, 2019

Copy link
Copy Markdown
Member

Closes #1702

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d. See documentation for details

@jaraco jaraco force-pushed the bugfix/1702-utf8-config branch from d23391d to e89509f Compare April 5, 2019 14:31
@benoit-pierre

Copy link
Copy Markdown
Member

Shouldn't the setuptools.unicode_utils.detect_encoding helper be removed?

@jaraco jaraco requested a review from benoit-pierre April 5, 2019 16:18
@jaraco

jaraco commented Apr 5, 2019

Copy link
Copy Markdown
Member Author

@pganssle I'd like to merge and release this today, so if you have any reservations, please let me know.

@pganssle

pganssle commented Apr 5, 2019

Copy link
Copy Markdown
Member

I don't have a major problem with the idea. My over-cautious instinct is to deprecate first, but this is such an easy thing to fix if it breaks something that maybe it's not worth it?

(Famous last words)

@jaraco

jaraco commented Apr 5, 2019

Copy link
Copy Markdown
Member Author

Good point. Here's my thinking: because the behavior was only recently released (January), probably not many project depend on the behavior or those that do are likely already cutting-edge enough to rely on this latest release. Moreover, if we find that releasing this does cause problems, I'll be on standby to release a bugfix release to add backward compatibility if needed (so only a small window of the problemmatic release).

@pganssle

pganssle commented Apr 5, 2019

Copy link
Copy Markdown
Member

Oh if you are reversing a recently introduced behavior then I think it's even less likely to cause problems, I say go for it.

@jaraco jaraco removed the request for review from benoit-pierre April 5, 2019 17:19
@jaraco

jaraco commented Apr 5, 2019

Copy link
Copy Markdown
Member Author

Thanks both. In the interest of progress, I'm going to move forward with this, but don't hesitate to raise other concerns or thoughts.

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.

4 participants