Skip to content

[MINOR][SS]Remove duplicate 'add' in comment of StructuredSessionization. #24589

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.

Sign up for GitHub

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

Jump to bottom
Closed
beliefer wants to merge 3 commits into from
Closed

[MINOR][SS]Remove duplicate 'add' in comment of StructuredSessionization. #24589

beliefer wants to merge 3 commits into from

Conversation

@beliefer
Copy link
Contributor

@beliefer beliefer commented May 13, 2019

What changes were proposed in this pull request?

StructuredSessionization comment contains duplicate 'add', I think it should be changed.

How was this patch tested?

Exists UT.

@beliefer beliefer changed the title [Minor][SS]Remove duplicate 'add' in comment of StructuredSessionization. [MINOR][SS]Remove duplicate 'add' in comment of StructuredSessionization. May 13, 2019
@SparkQA
Copy link

SparkQA commented May 13, 2019

Test build #105347 has finished for PR 24589 at commit f742f82.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

gaborgsomogyi
gaborgsomogyi approved these changes May 13, 2019
View reviewed changes
Copy link
Contributor

@gaborgsomogyi gaborgsomogyi left a comment

Choose a reason for hiding this comment

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

LGTM.

@HyukjinKwon
Copy link
Member

HyukjinKwon commented May 13, 2019

It's fine. but mind taking another look and see if there are some more typos to fix? I am sure there are more.

HeartSaVioR
HeartSaVioR approved these changes May 13, 2019
View reviewed changes
Copy link
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

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

LGTM

@beliefer
Copy link
Contributor Author

beliefer commented May 14, 2019

It's fine. but mind taking another look and see if there are some more typos to fix? I am sure there are more.

Thank you. I found another two typos, but I'm not sure.

@SparkQA
Copy link

SparkQA commented May 14, 2019

Test build #105371 has finished for PR 24589 at commit 2d1c9f6.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

HyukjinKwon
HyukjinKwon reviewed May 14, 2019
View reviewed changes
examples/src/main/scala/org/apache/spark/examples/sql/streaming/StructuredSessionization.scala Outdated
Copy link
Member

@HyukjinKwon HyukjinKwon May 14, 2019

Choose a reason for hiding this comment

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

Hm, other two occurrences are fine.

I'll just get this in - let's just keep the and typo fix only in this PR

-    // Sessionize the events. Track number of events, start and end timestamps of session, and
+    // Sessionize the events. Track number of events, start and end timestamps of session,

Copy link
Contributor Author

@beliefer beliefer May 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, other two occurrences are fine.

I'll just get this in - let's just keep the and typo fix only in this PR

-    // Sessionize the events. Track number of events, start and end timestamps of session, and
+    // Sessionize the events. Track number of events, start and end timestamps of session,

I find another type timestamps and changed to time stamps.
Is Sessionize a typo?

@SparkQA
Copy link

SparkQA commented May 14, 2019

Test build #105378 has finished for PR 24589 at commit c6f3aab.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

HyukjinKwon
HyukjinKwon reviewed May 14, 2019
View reviewed changes
examples/src/main/scala/org/apache/spark/examples/sql/streaming/StructuredSessionization.scala Outdated
}

// Sessionize the events. Track number of events, start and end timestamps of session, and
// Sessionize the events. Track number of events, start and end time stamps of session,
Copy link
Member

@HyukjinKwon HyukjinKwon May 14, 2019

Choose a reason for hiding this comment

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

I believe timestamp is fine ..

Copy link
Contributor Author

@beliefer beliefer May 14, 2019

Choose a reason for hiding this comment

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

I believe timestamp is fine ..

Is Sessionize a typo?

Copy link
Member

@srowen srowen May 14, 2019

Choose a reason for hiding this comment

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

Not a typo. Just remove 'and' here

Copy link
Contributor Author

@beliefer beliefer May 15, 2019

Choose a reason for hiding this comment

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

OK. I have recovered timestamps.

@SparkQA
Copy link

SparkQA commented May 15, 2019

Test build #105403 has finished for PR 24589 at commit e1f5039.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

HyukjinKwon pushed a commit that referenced this pull request May 15, 2019
@beliefer @HyukjinKwon
[MINOR][SS] Remove duplicate 'add' in comment of `StructuredSessioniz…
fbd2eac 
…ation`.

## What changes were proposed in this pull request?

`StructuredSessionization` comment contains duplicate 'add', I think it should be changed.

## How was this patch tested?

Exists UT.

Closes #24589 from beliefer/remove-duplicate-add-in-comment.

Lead-authored-by: gengjiaan <[email protected]>
Co-authored-by: Jiaan Geng <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
(cherry picked from commit 7dd2dd5)
Signed-off-by: HyukjinKwon <[email protected]>
@HyukjinKwon
Copy link
Member

HyukjinKwon commented May 15, 2019

Merged to master, branch-2.4 and branch-2.3

HyukjinKwon pushed a commit that referenced this pull request May 15, 2019
@beliefer @HyukjinKwon
[MINOR][SS] Remove duplicate 'add' in comment of `StructuredSessioniz…
306ebb2 
…ation`.

## What changes were proposed in this pull request?

`StructuredSessionization` comment contains duplicate 'add', I think it should be changed.

## How was this patch tested?

Exists UT.

Closes #24589 from beliefer/remove-duplicate-add-in-comment.

Lead-authored-by: gengjiaan <[email protected]>
Co-authored-by: Jiaan Geng <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
(cherry picked from commit 7dd2dd5)
Signed-off-by: HyukjinKwon <[email protected]>
@beliefer
Copy link
Contributor Author

beliefer commented May 15, 2019

@HyukjinKwon @srowen Thanks all your help! @gaborgsomogyi @HeartSaVioR Thanks all your review.

rluta pushed a commit to rluta/spark that referenced this pull request Sep 17, 2019
@beliefer
[MINOR][SS] Remove duplicate 'add' in comment of `StructuredSessioniz…
26d6cc9 
…ation`.

## What changes were proposed in this pull request?

`StructuredSessionization` comment contains duplicate 'add', I think it should be changed.

## How was this patch tested?

Exists UT.

Closes apache#24589 from beliefer/remove-duplicate-add-in-comment.

Lead-authored-by: gengjiaan <[email protected]>
Co-authored-by: Jiaan Geng <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
(cherry picked from commit 7dd2dd5)
Signed-off-by: HyukjinKwon <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Sep 26, 2019
@beliefer @kai-chi
[MINOR][SS] Remove duplicate 'add' in comment of `StructuredSessioniz…
6fd2ea8 
…ation`.

## What changes were proposed in this pull request?

`StructuredSessionization` comment contains duplicate 'add', I think it should be changed.

## How was this patch tested?

Exists UT.

Closes apache#24589 from beliefer/remove-duplicate-add-in-comment.

Lead-authored-by: gengjiaan <[email protected]>
Co-authored-by: Jiaan Geng <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
(cherry picked from commit 7dd2dd5)
Signed-off-by: HyukjinKwon <[email protected]>
@beliefer beliefer deleted the remove-duplicate-add-in-comment branch December 6, 2019 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srowen srowen srowen left review comments

@HyukjinKwon HyukjinKwon HyukjinKwon left review comments

@HeartSaVioR HeartSaVioR HeartSaVioR approved these changes

@gaborgsomogyi gaborgsomogyi gaborgsomogyi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@beliefer @SparkQA @HyukjinKwon @srowen @HeartSaVioR @gaborgsomogyi