Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upRepair log file encoding on module load if required #245
Conversation
Thanks for doing this. A number of items for you to consider. I also recommend that you come up with some manual test cases, list them out in your PR description, and validate that this change works across all of those tests cases (since we won't have UT's for it). |
@HowardWolosky thank you for the comments so far. I have yet to test, I just wanted to create a PR to show some progress. I do have a question in regards to how you prefer PRs. Should I only create the PR after everything is tested and functioning or is it alright to create a draft PR just to show progress? I don't want to waste your time always if it's not complete but want to show progress, and also in case there could be valuable input to give along the way? |
Thanks for asking! Draft PR's that are just showing progress are totally ok, however it's important to be clear about the state that it's in when you submit it. So, if you're submitting it and haven't tested/verified it yet, please just explicitly say so. If you're not looking for a thorough code review and just want general confirmation that you're on the right track, say that. Just be clear with why you're submitting the PR, and that will tell me how thoroughly I should be looking at it. Thanks for the help! |
@HowardWolosky Thank you for the information! |
Pending testing |
I will let you know once I've completed testing |
Sounds good. The code is looking pretty solid now, and I look forward to seeing your testing strategy. |
Apologies for the delay, I was wrapping up my last 2 weeks of internship. I'll get on this hopefully shortly. |
Hey @themilanfan -- Just wanted to check-in with you to see how things are going here. Hope your internship went well! Do you think you'll be coming back to this PR anytime soon? |
Hey @themilanfan -- wanted to check-in to see if you think you'll be coming back to this PR? Thanks! |
Description
Repair log file encoding from UTF16 to UTF8 when the module is loaded if required
Issues Fixed
Fixes #237
Checklist