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

Renames Add-CloudEvent*Data to Set-CloudEvent*Data #12

Merged

Conversation

dmilov
Copy link
Contributor

@dmilov dmilov commented Apr 9, 2021

  • Addresses issue Rename Add-CloudEvent*Data functions to Set-CloudEvent*Data #9 renaming Add-CloudEvent*Data functions to Set-CloudEvent*Data. As described in the issue the Add notion is not correct for the functions functionality. Set verb notion matches the behavior of the functions.
  • Increment minor version of the module. The module is not yet released but this is a backward compatibility breaking change.
  • Remove module catalog file creation from build.ps1 because catalog file is needed only when module is signed. We still don't have a signing implementation for this module.
  • Testing done:
> .\build.ps1
[11:24:08 AM] INFO: Publish CloudEvents.Sdk Module to 'C:\git-repos\dmilov-forks\sdk-powershell\CloudEvents.Sdk'
Microsoft (R) Build Engine version 16.8.3+39993bd9d for .NET
Copyright (C) Microsoft Corporation. All rights reserved.
                                                                                                                    Determining projects to restore...                                                                                All projects are up-to-date for restore.                                                                          CloudEventsPowerShell -> C:\git-repos\dmilov-forks\sdk-powershell\src\CloudEventsPowerShell\bin\Release\netstandard2.0\CloudEventsPowerShell.dll
  CloudEventsPowerShell -> C:\git-repos\dmilov-forks\sdk-powershell\CloudEvents.Sdk\
[11:24:10 AM] INFO: Run unit tests

Starting discovery in 9 files.
Discovery finished in 342ms.
[+] C:\git-repos\dmilov-forks\sdk-powershell\test\unit\ConvertFrom-HttpMessage.Tests.ps1 1.42s (421ms|812ms)
[+] C:\git-repos\dmilov-forks\sdk-powershell\test\unit\ConvertTo-HttpMessage.Tests.ps1 698ms (219ms|462ms)
[+] C:\git-repos\dmilov-forks\sdk-powershell\test\unit\New-CloudEvent.Tests.ps1 380ms (29ms|340ms)
[+] C:\git-repos\dmilov-forks\sdk-powershell\test\unit\Read-CloudEventData.Tests.ps1 356ms (17ms|324ms)
[+] C:\git-repos\dmilov-forks\sdk-powershell\test\unit\Read-CloudEventJsonData.Tests.ps1 384ms (44ms|327ms)
[+] C:\git-repos\dmilov-forks\sdk-powershell\test\unit\Read-CloudEventXmlData.Tests.ps1 439ms (68ms|343ms)
[+] C:\git-repos\dmilov-forks\sdk-powershell\test\unit\Set-CloudEventData.Tests.ps1 542ms (66ms|460ms)
[+] C:\git-repos\dmilov-forks\sdk-powershell\test\unit\Set-CloudEventJsonData.Tests.ps1 361ms (21ms|328ms)
[+] C:\git-repos\dmilov-forks\sdk-powershell\test\unit\Set-CloudEventXmlData.Tests.ps1 385ms (55ms|316ms)
Tests completed in 5.02s
Tests Passed: 28, Failed: 0, Skipped: 0 NotRun: 0
[11:24:17 AM] INFO: Run integration tests

Starting discovery in 1 files.
Discovery finished in 217ms.
[+] C:\git-repos\dmilov-forks\sdk-powershell\test\integration\HttpIntegration.Tests.ps1 2.84s (1.89s|757ms)
Tests completed in 2.86s
Tests Passed: 5, Failed: 0, Skipped: 0 NotRun: 0

SimeonGerginov
SimeonGerginov previously approved these changes Apr 9, 2021
Copy link

@SimeonGerginov SimeonGerginov left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -98,4 +98,3 @@ if (Test-Path $catalogFilePath) {
# Delete previous catalog file
Remove-Item $catalogFilePath -Confirm:$false
}
New-FileCatalog -Path $OutputDir -CatalogFilePath $catalogFilePath | Out-Null

Choose a reason for hiding this comment

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

Optional suggestion: Maybe we can remove the whole step Prepare Module for Publishing although the condition for removing the previous catalog file will never be invoked with the removal of the NewFileCatalog call.

src/CloudEventsPowerShell/CloudEvents.Sdk.psd1 Outdated Show resolved Hide resolved
src/CloudEventsPowerShell/CloudEvents.Sdk.psm1 Outdated Show resolved Hide resolved

.DESCRIPTION
This function adds data to a cloud event object with the provided parameters.
This function sets data into a cloud event object with the provided parameters.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This function sets data into a cloud event object with the provided parameters.
This function sets data in a cloud event object with the provided parameters.

src/CloudEventsPowerShell/CloudEvents.Sdk.psm1 Outdated Show resolved Hide resolved
src/CloudEventsPowerShell/CloudEvents.Sdk.psm1 Outdated Show resolved Hide resolved
src/CloudEventsPowerShell/CloudEvents.Sdk.psm1 Outdated Show resolved Hide resolved
test/unit/Set-CloudEventJsonData.Tests.ps1 Show resolved Hide resolved
Copy link
Member

@embano1 embano1 left a comment

Choose a reason for hiding this comment

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

LGTM, do you want to squash or squash during merge?

@dmilov
Copy link
Contributor Author

dmilov commented Apr 13, 2021

LGTM, do you want to squash or squash during merge?

I'll squash on merge

@dmilov dmilov merged commit d305f16 into cloudevents:main Apr 13, 2021
@dmilov dmilov deleted the topic/dmilov/rename-add-data-to-set-data branch April 26, 2021 05:37
@embano1 embano1 mentioned this pull request May 7, 2021
10 tasks
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