v13: Update deploy types (add setter to ArtifactDependency.Checksum and TryGetValue()/GetEntityTypes() to IFileTypeCollection)

[ronaldbarendse]

PR #15144 added the Checksum property to ArtifactDependency to be able to remove the custom JsonConverter workaround in Deploy 13 that (de)serializes this property to/from the artifact JSON. The checksums are only calculated/populated when doing an export and without the workaround we don't need to create a new instance of a subclassed type anymore, so we should actually make the property settable/writable 😄

The IFileTypeCollection implementation in Deploy now also requires being able to return all registered entity types in GetEntityTypes() and we've added a TryGetValue() method for a more performant lookup. These methods are now added to the interface.

Finally, I've updated all deploy related XML docs/comments, because some didn't have any or were not really explaining the concepts correctly.

[AndyButland]

Looks fine to me, and as you say, is mostly comments and cleanup.

[nul800sebastiaan]

Looks good to me! 👍

[bergmania]

@nul800sebastiaan, I think we should revert this, if it cant be made non-breaking. The final release is too near IMO

[nul800sebastiaan]

Hopefully should be solved by providing a default implementation for the interface additions #15363

[bergmania]

it's not really fixing the problem. If people implemented the old interface, but our code now calls these new methods it will throw.

[nul800sebastiaan]

@bergmania This is how we encourage other contributors to do it, and we've merged quite a few PRs that way in the past year, should we stop doing this altogether then?

[bergmania]

The biggest issue here is timing.. Breaking changes of existing code should have been part of the first RC so package developers have better odds for making their packages compatible.

Default interface implementations are fine in some case, but just throwing an exception still makes it break at runtime, but effectively hides it for developers on build time.

In cases where a sensible default implementation cannot be made (e.g calling another method from the interface), I think it is a better solution to have the new methods on a new interface and let both be implemented. Then the using code can check if it is implemented.

[nul800sebastiaan]

discussing with @ronaldbarendse now as well, things are clarifying for me - he's updated the PR with a much more reasonable response for those methods

[nul800sebastiaan]

Merged #15363 to unbreak it!