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

Updated to document breaking changes #762

Merged
Prev Previous commit
Next Next commit
Removed bullet
  • Loading branch information
Maryanne Njeri committed Aug 1, 2023
commit b9fdfcf744b0ddec55960393a011c12e7fd46cd7
2 changes: 1 addition & 1 deletion releaseNotes/AzureDevOpsPushPipelineExtension.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Azure App Configuration Push extension for Azure DevOps pipeline can be installe
![sample](pictures/AzureDevOpsPushExtensionVersionSample.PNG)

### v5.0.0 - February, 02 2023
* **Breaking Changes**:
**Breaking Changes**:
- Updated the task to require Node.js 16. It previously required 10.
- Updated the minimum supported azure pipeline agent version to [2.206.1](https://github.com/microsoft/azure-pipelines-agent/releases/tag/v2.206.1) or later. Previously it was [2.144.0](https://github.com/microsoft/azure-pipelines-agent/releases/tag/v2.144.0).
- Importing key vault references value as an escaped json strings is not supported, please specify the key vault reference value as a json object.
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 a change for all files using application/json content type right, not just kv set? If so, we'll want to call that out.

Also, may help to provide a snippet of what has changed.

Something like

Before when you had this

bla bla

you got this

bla bla bla

Now, the input needs to be

bla bla bla

Copy link
Contributor Author

@MaryanneNjeri MaryanneNjeri Oct 13, 2023

Choose a reason for hiding this comment

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

It only applies for default content profile, using application/json content type.
How about something like :
"Importing configurations with values as escaped json strings and the content type "application/json" is no longer supported. Please specify value as a json object."

Before

{  
  "app": "{\"uri\":\"https://keyvault.vault.azure.net/secrets/secret\"}"
}

Results:

Key Value
app {"uri":"https://keyvault.vault.azure.net/secrets/secret"}

After

{  
  "app": {
      "uri": "https://keyvault.vault.azure.net/secrets/secret " 
     }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking since the breaking change is in relation to importing configurations, we can show the before and after of the snipped that has changed and skip showing the results

Copy link
Member

Choose a reason for hiding this comment

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

How about something like :
"Importing configurations with values as escaped json strings and the content type "application/json" is no longer supported. Please specify value as a json object."

I would avoid the phrase "is no longer supported." It's not that we removed support but rather we changed the behavior. We now convert the values of properties in the settings to json strings.

I was thinking since the breaking change is in relation to importing configurations, we can show the before and after of the snipped that has changed and skip showing the results

Sounds reasonable. But, I would include the content type that goes with the associated input snippet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would avoid the phrase "is no longer supported." It's not that we removed support but rather we changed the behavior. We now convert the values of properties in the settings to json strings.

Yes good point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been updated thanks

Expand Down