-
Notifications
You must be signed in to change notification settings - Fork 183
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
Restructure Changelog Logic #823
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,17 +3,11 @@ param ( | |
[String]$ChangeLogLocation, | ||
[String]$VersionString, | ||
[string]$PackageName, | ||
[string]$ServiceName, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These changes will break any consumers so somehow we need to figure out a way of not breaking them. One potential way is to make changes additive and then go back in after you moved everyone to the new approach and remove the unused parameters. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course this is only an issue if there is anyone directly calling this script. If they are all using the yml template you can do what I suggested in my comment in the template to handle the break. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided to go with making the changes additive. So I added the new files and will switch then over in the languages repo so I can make sure it all works and there is no tome where things are broken. The only down side is that I have appended There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I personally wouldn't create a new template but instead just make the changes to the existing template additive like I suggested. If you add the second template you will have the same problem when you want to clean that script up. |
||
[string]$RepoRoot, | ||
[ValidateSet("net", "java", "js", "python")] | ||
[string]$Language, | ||
[string]$RepoName, | ||
[string]$ServiceDirectory, | ||
[boolean]$ForRelease = $False | ||
) | ||
|
||
$ProgressPreference = "SilentlyContinue" | ||
. (Join-Path $PSScriptRoot SemVer.ps1) | ||
Import-Module (Join-Path $PSScriptRoot modules ChangeLog-Operations.psm1) | ||
. (Join-Path $PSScriptRoot common.ps1) | ||
|
||
$validChangeLog = $false | ||
if ($ChangeLogLocation -and $VersionString) | ||
|
@@ -22,22 +16,8 @@ if ($ChangeLogLocation -and $VersionString) | |
} | ||
else | ||
{ | ||
Import-Module (Join-Path $PSScriptRoot modules Package-Properties.psm1) | ||
if ([System.String]::IsNullOrEmpty($Language)) | ||
{ | ||
if ($RepoName -match "azure-sdk-for-(?<lang>[^-]+)") | ||
{ | ||
$Language = $matches["lang"] | ||
} | ||
else | ||
{ | ||
Write-Error "Failed to set Language automatically. Please pass the appropriate Language as a parameter." | ||
exit 1 | ||
} | ||
} | ||
|
||
$PackageProp = Get-PkgProperties -PackageName $PackageName -ServiceName $ServiceName -Language $Language -RepoRoot $RepoRoot | ||
$validChangeLog = Confirm-ChangeLogEntry -ChangeLogLocation $PackageProp.pkgChangeLogPath -VersionString $PackageProp.pkgVersion -ForRelease $ForRelease | ||
$PackageProp = Get-PkgProperties -PackageName $PackageName -ServiceDirectory $ServiceDirectory | ||
$validChangeLog = Confirm-ChangeLogEntry -ChangeLogLocation $PackageProp.ChangeLogPath -VersionString $PackageProp.Version -ForRelease $ForRelease | ||
} | ||
|
||
if (!$validChangeLog) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps as a transition you have both ServiceName and ServiceDirectory and you do something like
coalesce(parameters.ServiceDirectory, parameters.ServiceName)
, and then once you have converted all the usages you can remove the ServiceName parameter.