Remove miq-unicode dependency, encode manually #6
Conversation
|
There is a module for utility methods, where the method might be more appropriate - https://github.com/ManageIQ/virtfs-ntfs/blob/master/lib/virtfs/ntfs/utils.rb I kind of like the code saying "UnicodeToUtf8", because it's clearer, so I think creating utility method in NTFS Utils is a better approach for readability. |
|
@Fryguy Ok, updated. |
| @@ -1,5 +1,3 @@ | |||
| require 'util/miq-unicode' | |||
There was a problem hiding this comment.
Not sure, but you might need a require 'fs/ntfs/utils' here like is done in the other places.
There was a problem hiding this comment.
@Fryguy Actually, where is this coming from?
There was a problem hiding this comment.
@Fryguy It looks to me like this library assumed manageiq-smartstate was a dependency, but it's not marked as such, and that's a different utils.rb file anyway.
There was a problem hiding this comment.
oh very interesting...I missed that detail
There was a problem hiding this comment.
The virtfs code was expected to be extracted from manageiq-smartstate. In fact, I'd expect you to run into duplicated code. Note that a lot of this code doesn't actually work yet, and was in progress, but has since been abandoned. It is not used at all by the application.
On that note, I think we should abandon this virtfs stuff for now (like completely abandon, archive the repos) @chessbyte
There was a problem hiding this comment.
@Fryguy I'm all in favor of archiving this if it's not actually being used.
There was a problem hiding this comment.
Seeing as these virt* repos have not been active for awhile and just cause noise (needless PRs and Issues) due to gem and cop conflicts, let's archive them for now. If someone wants to work on them in earnest, we can always un-archive them. /cc @roliveri
Put method into Utils module, use that. Add AsciiToUtf8 method to Utils and use it. Remove pointless utf8 conversion. Co-authored-by: Jason Frey <[email protected]> Remove AsciiToUtf8, not going to use it after all.
|
Closing since we are archiving |
Part of the gems-pending cleanup effort. Since there are only a handful of references, I simply replaced them with the longhand version.
Note that I didn't use
.duphere since they're already substrings (or in one case, a constant assignment).Followup to ManageIQ/manageiq-smartstate#149