Skip to content

Update Language-Settings for new moduleVersion#16801

Closed
seankane-msft wants to merge 2 commits intomainfrom
seankane-msft-patch-2
Closed

Update Language-Settings for new moduleVersion#16801
seankane-msft wants to merge 2 commits intomainfrom
seankane-msft-patch-2

Conversation

@seankane-msft
Copy link
Contributor

autorest changed the variable name from version to moduleVersion

  • The purpose of this PR is explained in this or a referenced issue.
  • The PR does not update generated files.
  • Tests are included and/or updated for code changes.
  • Updates to CHANGELOG.md are included.
  • MIT license headers are included in each file.

autorest changed the variable name from `version` to `moduleVersion`
Copy link
Member

@chidozieononiwu chidozieononiwu left a comment

Choose a reason for hiding this comment

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

You'll also need to update

return "$($matches["version"])", $versionFile

{
$NO_PREFIX_VERSION_LINE_REGEX = ".+\s*=\s*`"(?<bad_version>$([AzureEngSemanticVersion]::SEMVER_REGEX))`""
$VERSION_LINE_REGEX = ".+\s*=\s*`".*v(?<version>$([AzureEngSemanticVersion]::SEMVER_REGEX))`""
$VERSION_LINE_REGEX = ".+\s*=\s*`".*v(?<moduleVersion>$([AzureEngSemanticVersion]::SEMVER_REGEX))`""
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to update line 24 below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thank you

function Get-GoModuleVersionInfo($modPath)
{
$NO_PREFIX_VERSION_LINE_REGEX = ".+\s*=\s*`"(?<bad_version>$([AzureEngSemanticVersion]::SEMVER_REGEX))`""
$VERSION_LINE_REGEX = ".+\s*=\s*`".*v(?<version>$([AzureEngSemanticVersion]::SEMVER_REGEX))`""
Copy link
Member

Choose a reason for hiding this comment

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

This is just changing the named group in a RegEx which isn't used outside of this regex and matching. Thus this is pretty much a noop, what is it that you are trying to update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The variable name has changed from version to moduleVersion, I am looking for the regex that searches for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at this more in depth, I don't think there is a check on the variable version. Is that correct @weshaggard ? It looks like it's doing a search for semantic versioning in a specific file.

Copy link
Member

@weshaggard weshaggard Jan 12, 2022

Choose a reason for hiding this comment

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

That is correct this regex is only looking for something that looks like "=v". If it finds that in one of the file names we look in hen it assumes that is the module version. If we trying to be more prescriptive then we would want to do something like moduleVersion\s*=\s*``".*v(?<version>$([AzureEngSemanticVersion]::SEMVER_REGEX))``"

If we do this we need to verify that all the existing versions currently match that or it will throw off a number of processes in this repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now I don't think we need to be more prescriptive. I am going to close this PR.

@seankane-msft seankane-msft deleted the seankane-msft-patch-2 branch January 24, 2022 21:46
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.

4 participants