Fix IsBinaryFile binary detection (#702)#716
Conversation
ziagham
left a comment
ziagham
left a comment
There was a problem hiding this comment.
Thank you for the clean refactoring of the IsBinaryFile method. However, please remove the unit test since it’s in the wrong project and location, and also remove the ProjectReferences from FlowSynx.UnitTests.csproj. In addition, please delete the unnecessary AssemblyInfo.cs file, as it shouldn’t be included—this isn’t the proper way to expose an internal class.
I will create a separate issue for adding unit tests for the LocalFileSystem plugin. For now, please commit only the changes related to IsBinaryFile and delete everything else.
|
Thanks @ziagham for pointing that out. I've removed the misplaced ConverterExtensions tests, dropped the FlowSynx.UnitTests project references, and deleted the Properties/AssemblyInfo.cs file so the PR now only includes the IsBinaryFile refactor. Let me know if anything else needs to change. |
|
@mohin-io Could you please check whether you committed your changes? The code still looks the same as before, and the latest commit is from two days ago. |
|
|
Hi @ziagham, I just pushed commit 5dfcabc to issue-702-fix-isbinary; it removes the misplaced ConverterExtensions tests, drops the LocalFileSystem project reference from FlowSynx.UnitTests.csproj, and deletes the temporary AssemblyInfo.cs, so the PR now only contains the IsBinaryFile refactor. Please check the latest commit and let me know if anything else looks off from my side |
2 participants
Summary
Closes #702
Testing