Skip to content

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

Merged
michaelnebel merged 2 commits into
github:mainfrom
michaelnebel:csharp/aligntelemetryimplementation
May 7, 2024
Merged

C#: Base telemetry Api Source/Sink nodes on abstract classes.#16356
michaelnebel merged 2 commits into
github:mainfrom
michaelnebel:csharp/aligntelemetryimplementation

Conversation

@michaelnebel

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions github-actions Bot added the C# label Apr 29, 2024
@michaelnebel

Copy link
Copy Markdown
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

@hvitved hvitved left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same

/**
* 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
Copy Markdown
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
@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.

2 participants