Validate workspace recommended extensions against the marketplace#40270
Validate workspace recommended extensions against the marketplace#40270ramya-rao-a merged 5 commits intomicrosoft:masterfrom sbatten:users/stbatt/wsextrec
Conversation
validation to Workspace Recommendations
ramya-rao-a
left a comment
ramya-rao-a
left a comment
There was a problem hiding this comment.
There is scope for a few improvements. Take a look.
| }); | ||
|
|
||
| if (filteredRecommendations.length !== extensionsContent.recommendations.length) { | ||
| let badlyFormattedRecommendations = extensionsContent.recommendations.filter(element => { |
There was a problem hiding this comment.
Instead of looping through extensionsContent.recommendations one more time here, you can use the callback for the extensionsContent.recommendations.filter to find the badly formatted recommendations and form the badRecommendationsString
| return extensionsContent.recommendations.indexOf(element) === position && regEx.test(element); | ||
| }); | ||
|
|
||
| if (filteredRecommendations.length !== extensionsContent.recommendations.length) { |
There was a problem hiding this comment.
This condition will pass if there are duplicate valid recommendations as well
| }); | ||
|
|
||
| let unknownExtensionsString = ''; | ||
| unknownExtensions.forEach(element => { |
There was a problem hiding this comment.
Why loop one more time, when you can use the callback for filteredRecommendations.filter to form the unknownExtensionsString
| }); | ||
|
|
||
| console.log(unknownExtensions.length + | ||
| ' extension(s) in workspace recommendations were not found in the gallery:\n' + |
There was a problem hiding this comment.
nit: Prefix this message with The below . Same for the other message
| badlyFormattedRecommendations.forEach(element => { | ||
| badRecommendationsString += element + '\n'; | ||
| }); | ||
| if (!regEx.test(element)) { |
There was a problem hiding this comment.
This way if there is a recommendation not following the regex appearing twice, then it will appear in the badRecommendationsString twice.
Duplication in recommendations doesn't cause any harm. We don't need to add a warning for it.
There was a problem hiding this comment.
I'm fixing this to only log once per distinct value as recommended. However, the reason I was in favor of logging duplicates is specifically for when the author has duplicate bad entries. They will see they have a bad entry, fix it, then see the exact same error logging and possibly be very confused if their changes were picked up. I could check to see if a bad entry is a duplicate (using lastIndexOf, and log as (bad format, duplicate)
| filteredRecommendations.forEach(element => { | ||
| if (validRecommendations.indexOf(element.toLowerCase()) === -1) { | ||
| countBadRecommendations++; | ||
| badRecommendationsString += `${element} (not found in gallery)\n`; |
There was a problem hiding this comment.
Use the word marketplace instead of gallery. I know, its called the gallery service, but the website users can go search for extension is called marketplace (powered by the same gallery service :) )
2 participants
Don't show message about workspace recommended extensions if none of the recommended extensions are valid (i.e. they are badly formatted or don't exist in the marketplace)