search

wix-incubator / corvid-types

Type definitions for Corvid by Wix
MIT License
5 stars 3 forks source link

Add getWixModulesList #33

Closed matanlb closed 4 years ago

nemni8 commented 4 years ago

looks good!

matanlb commented 4 years ago

Also - what do you think about adding a small test for this? In its current form, it may be a rabit problematic because the code looks specifically for the corvid-declarations, but imagine if you extracted the logic to a lib-style function that would accept a declaration file and a target JSON and it would extract the module list to that JSON? that would be easy to unit test, and might make the code simpler. I don't think it's a big refactor, and the added benefit of having a test might be worth it. WDYT?

E.g something like extractModuleNames(declarationFile) or writeModuleNamesToFile(declarationFile, targetFile) Personally, I like the first option better.

I do want to add tests to it but I don't want to transform this a script into a library just for the sake of testing the internals, I think I would prefer testing the outputted artifact to see that it is correct, perhaps by getting the names in a more direct way from the file