Skip to content

VS-156: Support Strong Named 2.28 Driver #84

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

Merged
merged 16 commits into from
Jul 29, 2024

Conversation

Ravi-Raghavan
Copy link
Collaborator

No description provided.

@Ravi-Raghavan Ravi-Raghavan marked this pull request as draft July 25, 2024 15:21
@Ravi-Raghavan Ravi-Raghavan marked this pull request as ready for review July 25, 2024 18:48
@Ravi-Raghavan Ravi-Raghavan requested a review from BorisDog July 25, 2024 20:12
Copy link
Collaborator

@BorisDog BorisDog left a comment

Choose a reason for hiding this comment

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

Few more changes requested.

@Ravi-Raghavan Ravi-Raghavan requested a review from BorisDog July 26, 2024 13:35
@@ -56,5 +60,7 @@ analysisType switch
AnalysisType.Poco => JsonSyntaxElements.Poco.JsonGeneratorFullName,
_ => throw new ArgumentOutOfRangeException(nameof(analysisType), analysisType, "Unsupported analysis type")
};

private static bool IsDriverVersion_2_28_Or_Greater(Version driverVersion) => driverVersion >= s_driverVersion_2_28;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's pretty simple condition, probably no need for a separate method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

PathUtilities.VerifyTestDataModelAssembly();
var isDriverVersion_2_28_OrGreater = PathUtilities.IsDriverVersion_2_28_OrGreater(driverVersion);
var testDataModelAssembly = isDriverVersion_2_28_OrGreater ? PathUtilities.TestDataModelAssemblyPathDriver_2_28_OrGreater : PathUtilities.TestDataModelAssemblyPathDriver_2_27_OrLower;
PathUtilities.VerifyTestDataModelAssembly(testDataModelAssembly);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe refactor all this to PathUtililites.GetTestDataModelAssemblyPath?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link
Collaborator

@BorisDog BorisDog left a comment

Choose a reason for hiding this comment

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

LGTM, good job!

syntaxTrees,
referencesContainer.References,
new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary));
new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, publicSign: true, cryptoPublicKey: s_publicKey.ToImmutableArray()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

make s_publicKey immutable array, to skip the conversion each time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@Ravi-Raghavan Ravi-Raghavan requested a review from BorisDog July 29, 2024 13:44
@Ravi-Raghavan Ravi-Raghavan merged commit dab5582 into mongodb:main Jul 29, 2024
23 checks passed
@Ravi-Raghavan Ravi-Raghavan deleted the VS-156 branch July 29, 2024 14:14
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.

3 participants