Add a new test case for leading dots in file extension declaration. #518
Conversation
|
@robertohuertasm Is there an issue with this PR or you want to do same more testing? |
|
I just want to find some time to review this once I get home 😉 |
|
@JimiC I think we don't need this. I mean, |
|
If you take a closer look filename cases are excluded. This test is on for those mark as extensions. |
|
Sorry @JimiC, I was misled by a previous comment from #510 and didn't pay proper attention:
I thought that when you said It's ok then 😀 |
| @@ -20,6 +20,16 @@ describe('generating icons', function () { | |||
| expect(iconGenerator.getPathToDirName(toDirName, fromDirPath)).toEqual(pathTo); | |||
| }); | |||
|
|
|||
| it('file extension should not have a leading dot', function () { | |||
| files.supported | |||
| .filter(function (file) { return !file.filename; }) | |||
There was a problem hiding this comment.
I guess that you've chosen to favor readability instead of performance by double looping over the files.supported collection, haven`t you?
There was a problem hiding this comment.
Yeap. The impact isn't noticeable.
Glossary:
Hope this helps us speaking a common |
| @@ -144,7 +154,7 @@ describe('generating icons', function () { | |||
| var fileExtensions = iconGenerator.buildJsonStructure().files.names.fileExtensions; | |||
|
|
|||
| files.supported | |||
| .filter(function (file) { return !file.filename && !file.languages; }) | |||
There was a problem hiding this comment.
This removal is due to the decision to include the language supported extension in the test.
At first I excluded them but after you clarified it I forgot to change it back.
There was a problem hiding this comment.
If everything is fine I'll proceed with the merge then.
There was a problem hiding this comment.
I just pointed this out in case you were wondering.
I'm a GO.
No description provided.