search

eyeonus / Trade-Dangerous

Mozilla Public License 2.0
97 stars 31 forks source link

Enforcing UTF-8 encoding when writing downloaded files #114

Closed c2technology closed 1 year ago

c2technology commented 1 year ago

Adds UTF-8 encoding when opening a file ensuring the file is encoded properly for parsing.

eyeonus commented 1 year ago

This PR fails during tox testing due to 'modules.json' failing to download correctly, most likely because changing the download from binary mode to UTF-8 encoding breaks things when the original file isn't encoded in UTF-8.

A more appropriate fix would be to ensure UTF-8 encoding when opening the problem file for importing into the database.

c2technology commented 1 year ago

yup didn't mean to submit this to the main repo. retracting

eyeonus commented 1 year ago

It seems you misunderstand how PRs work.

I put your PR through standard testing, and your code fails that testing. I explained to you what failed and my guess as to the cause.

Because it failed standard testing, I have not accepted the PR. Unless one of the maintainers, such as myself, accepts a Pull Request, it will not be merged into the main branch of this repository.

I did not close the PR when I commented on this because you may add additional commits to your PR branch to fix the problems in it so that it can then be accepted and merged.

I am at work, replying by mail. If you would like to have your encoding fix incorporated, I encourage you to fix your problem in a way that doesn't break something else, and reopen this PR once you've committed those changes to this.

On Wed, Apr 26, 2023, 19:29 Chris Ryan @.***> wrote:

yup didn't mean to submit this to the main repo. retracting

— Reply to this email directly, view it on GitHub https://github.com/eyeonus/Trade-Dangerous/pull/114#issuecomment-1524386600, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANHHYFZCEUG2Y7GTUBODQDXDHDWXANCNFSM6AAAAAAXM2KRMA . You are receiving this because you commented.Message ID: @.***>