Skip to content

Conversation

@multiarc
Copy link
Owner

No description provided.

@multiarc multiarc requested a review from Copilot July 12, 2025 02:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades serializer-related package versions to their latest releases and refactors benchmark setup to centralize serializer instantiation.

  • Updated package-lock and csproj references for all serializers
  • Introduced DataItemMemoryPack and extension method for MemoryPack support
  • Refactored benchmarks to use a new CreateSerializerInstance factory method

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
SerializersBenchmark/packages.lock.json Updated resolved versions and content hashes for all packages
SerializersBenchmark/SerializersBenchmark.csproj Bumped PackageReference versions to match lock file upgrades
SerializersBenchmark/Models/DataItemMemoryPack.cs Added new MemoryPackable model for MemoryPack benchmarks
SerializersBenchmark/Models/DataItem.cs Adjusted MessagePackObject attribute and removed outdated MemoryPack annotations
SerializersBenchmark/Models/CreateDataExtensions.cs Added DataMemoryPack method to create DataItemMemoryPack instances
SerializersBenchmark/Benchmarks.cs Replaced manual Activator.CreateInstance logic with CreateSerializerInstance
SerializersBenchmark/Base/SerializerFactoryExtensions.cs Added factory extension to map serializer types to their data creators
SerializersBenchmark/AsyncBenchmarks.cs Updated async setup to use the new factory extension
Comments suppressed due to low confidence (4)

SerializersBenchmark/Base/SerializerFactoryExtensions.cs:8

  • [nitpick] Consider adding XML doc comments to explain the purpose, parameters, and return value of CreateSerializerInstance for improved readability and maintainability.
    public static ISerializerTestAsync CreateSerializerInstance(this Type serializerType) {

SerializersBenchmark/Models/CreateDataExtensions.cs:22

  • The new DataMemoryPack method introduces a MemoryPack-specific data path but lacks unit tests. Add tests to verify serialization and deserialization correctness for DataItemMemoryPack.
    public static DataItemMemoryPack DataMemoryPack(int itemsToCreate)

SerializersBenchmark/Models/DataItemMemoryPack.cs:9

  • The class uses [DataMember] annotations but lacks a class-level [DataContract] attribute, so some serializers (e.g., DataContractSerializer) may ignore those members. Consider adding [DataContract] above the class declaration.
[MemoryPackable]

SerializersBenchmark/Base/SerializerFactoryExtensions.cs:10

  • [nitpick] The if-else chain in CreateSerializerInstance may grow unwieldy as new serializers are added. Consider using a dictionary or strategy pattern to map serializer types to factory delegates.
        if (serializerType == typeof(GoogleProtobuf<ProtobufDataItem>)) {

@multiarc multiarc merged commit eb06dbb into main Jul 12, 2025
3 checks passed
@multiarc multiarc deleted the packages/refresh branch July 12, 2025 02:13
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.

2 participants