Skip to content

Conversation

Niharikadutta
Copy link
Collaborator

@Niharikadutta Niharikadutta commented Mar 31, 2020

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

@imback82 imback82 changed the title Adding guides to explain UDF serialization and Broadcast variable usage [DOC] Adding guides to explain UDF serialization and Broadcast variable usage Mar 31, 2020
@Niharikadutta
Copy link
Collaborator Author

@bamurtaugh Could you please take a look at the following guides I wrote to explain the UDF serialization behavior and Broadcast variable usage? Thank you for your time!

Copy link
Member

@bamurtaugh bamurtaugh left a comment

Choose a reason for hiding this comment

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

These are great, thanks so much for working on them @Niharikadutta 😄

@Niharikadutta
Copy link
Collaborator Author

@imback82 , @bamurtaugh Have added a section for UDF serialization and addressed all remaining comments. Please feel free to take a look and let me know your thoughts. Thanks!

bamurtaugh
bamurtaugh previously approved these changes Apr 20, 2020
Copy link
Member

@bamurtaugh bamurtaugh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @Niharikadutta!

@Niharikadutta
Copy link
Collaborator Author

@imback82 Please let me know if this looks ok or if there are any changes/suggestions you have. Thanks!

Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

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

few nit comments, but LGTM in general.

@Niharikadutta
Copy link
Collaborator Author

@imback82 addressed remaining comments, please let me know if it looks ok. Thanks!

@imback82 imback82 merged commit 08d203a into dotnet:master May 27, 2020
@Niharikadutta Niharikadutta deleted the nidutta/UdfAndBroadcastGuides branch August 24, 2020 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants