docs(v3): add v2.x-v3 migration guide #501
Conversation
✅ Deploy Preview for arcus-security ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
fgheysels
left a comment
fgheysels
left a comment
There was a problem hiding this comment.
Thanks for this.
I have some suggestions/remarks though :)
| * 🗑️ .NET 6 support is removed. All Arcus.Security.* packages support .NET 8 and stop supporting .NET 6. (.NET 10 support starts from v2.1.) | ||
| * 🗑️ Transient GuardNET dependency is replaced by built-in argument checking. | ||
| * 🗑️ Transient Arcus.Observability dependency for auditing is removed. | ||
| * ✏️ The new main core types are now under the `Arcus.Security` namespace, instead of previously the `Arcus.Security.Core` namespace. Following types in the old namespace are removed: |
There was a problem hiding this comment.
I think it would be more clear if we explicitly state (repeat) the name of the namespace instead of saying (... in the old namespace are removed).
| * `MutatedSecretNameCachedSecretProvider` | ||
|
|
||
| ## 🎯 Use `ISecretStore` instead of `ISecretProvider` as the secret store's main point of contact | ||
| Starting from v3, accessing the secret store now happens via the `Arcus.Security.ISecretStore` interface (in new namespace), instead of previously using the same `Arcus.Security.Core.ISecretProvider` interface as for the external secret provider implementations. |
There was a problem hiding this comment.
The namespace is mentioned in the typename, so is it necessary to add the '(in new namespace)' here ?
Hmm, maybe to emphasize that the change is in deed in the namespacename.
There was a problem hiding this comment.
Yeah, would be in favor of a full namespace here to show the different location.
| ```csharp | ||
| string secretValue = await store.GetSecretAsync("<name>"); | ||
| ``` | ||
| Just be aware that in case of a failure, an exception will still be thrown. |
There was a problem hiding this comment.
Maybe we should've implemented that in such a way that we return null instead of throwing an exception ?
There was a problem hiding this comment.
Might be better communicated to users that it is only there if there is a result, otherwise we could have succes with null or not found being rather similar.
Also following the built in result type in F# here as well.
| ``` | ||
|
|
||
| :::info | ||
| The `SecretsResult` acts the same way as the `SecretResult`, only for a collection of secrets (implements `IEnumerable<SecretResult>`). |
There was a problem hiding this comment.
Now that I read this, maybe we should've named it SecretResultCollection instead of SecretsResult ?
There was a problem hiding this comment.
It is more a result than a collection of results, though. It's a folded/aggregated result.
Co-authored-by: Frederik Gheysels <[email protected]>
Co-authored-by: Frederik Gheysels <[email protected]>
2 participants
Adds the initial proposal for the migration guide to be included in the v2.1 release so that migration from deprecated members/types to the new ones happen more smoothly before the actual v3 breaking change release.