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

C#: Base telemetry Api Source/Sink nodes on abstract classes. #16356

Merged

Conversation

michaelnebel
Copy link
Contributor

No description provided.

@github-actions github-actions bot added the C# label Apr 29, 2024
@michaelnebel
Copy link
Contributor Author

DCA looks good.
This is to align the C# implementation with the Java implementation: #16297

@michaelnebel michaelnebel marked this pull request as ready for review April 30, 2024 07:59
@michaelnebel michaelnebel requested a review from a team as a code owner April 30, 2024 07:59
@michaelnebel michaelnebel added the no-change-note-required This PR does not need a change note label Apr 30, 2024
@michaelnebel michaelnebel force-pushed the csharp/aligntelemetryimplementation branch from 76aed36 to e64a2d6 Compare May 3, 2024 10:49
hvitved
hvitved previously approved these changes May 6, 2024
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM (two typos).


/**
* Module that adds all API like sinks to `SinkNode`, excluding sinks for cryptography based
* queries, and queries where sinks are not succifiently defined (eg. using broad method name matching).
Copy link
Contributor

Choose a reason for hiding this comment

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

typo


/**
* Module that adds all API like sources to `SourceNode`, excluding some sources for cryptography based
* queries, and queries where sources are not succifiently defined (eg. using broad method name matching).
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@@ -6,7 +6,7 @@ final class SourceNode = FlowSources::SourceNode;

/**
* Module that adds all API like sources to `SourceNode`, excluding some sources for cryptography based
* queries, and queries where sources are not succifiently defined (eg. using broad method name matching).
* queries, and queries where sources are not ntly defined (eg. using broad method name matching).
Copy link
Contributor

Choose a reason for hiding this comment

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

Not quite fixed ;-)

@michaelnebel michaelnebel force-pushed the csharp/aligntelemetryimplementation branch from ba7e827 to 757cf8d Compare May 6, 2024 14:48
@michaelnebel michaelnebel merged commit cfb0a86 into github:main May 7, 2024
20 checks passed
@michaelnebel michaelnebel deleted the csharp/aligntelemetryimplementation branch May 7, 2024 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants