Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support wild cards in nuget versions for dialog:merge #1096

Merged
merged 4 commits into from Jan 27, 2021
Merged

Conversation

chrimc62
Copy link
Contributor

This fixes composer bug: microsoft/BotFramework-Composer#5576 by adding support for matching nuget versions with * by building a regex and matching against the file path names.

Copy link
Contributor

@joshgummersall joshgummersall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple suggestions, otherwise LGTM.

@@ -1069,18 +1094,27 @@ export class SchemaMerger {
for (const pkgVersion of await fs.readdir(pkgPath)) {
versions.push(pkgVersion.toLowerCase())
}
// NOTE: The semver package does not handle more complex nuget range revisions
// We get an exception and will ignore those dependencies.
let versionToUse: any
Copy link
Contributor

@joshgummersall joshgummersall Jan 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any should almost always be avoided if possible. #Resolved

if (this.debug) {
this.parsingWarning(`Using most recent version ${minVersion}`)
}
} else if (minVersion.includes('*')) {
// Match pattern against available versions
let pattern = this.nugetPattern(minVersion)
Copy link
Contributor

@joshgummersall joshgummersall Jan 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const #Resolved

@@ -1055,6 +1056,30 @@ export class SchemaMerger {
}
}

// Convert a nuget pattern to a regexp
Copy link
Contributor

@joshgummersall joshgummersall Jan 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment that includes a few examples of nuget versions and the expected RegExp could be helpful. #Resolved

@@ -39,7 +39,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="nuget1" Version="10.0.0" />
<PackageReference Include="nuget1" Version="10.*" />
Copy link
Member

@GeoffCoxMSFT GeoffCoxMSFT Jan 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be worth putting in some with * in other version parts plus having the format from the daily builds where there is more text in different parts. example: 4.12.0-daily.* #WontFix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants