Skip to content

[MINOR][CORE][TESTS] Fix EventLogFileWritersSuite typo#43895

Closed
surjikal wants to merge 1 commit intoapache:masterfrom
surjikal:SPARK-34503
Closed

[MINOR][CORE][TESTS] Fix EventLogFileWritersSuite typo#43895
surjikal wants to merge 1 commit intoapache:masterfrom
surjikal:SPARK-34503

Conversation

@surjikal
Copy link
Contributor

@surjikal surjikal commented Nov 20, 2023

What changes were proposed in this pull request?

A simple typo fix

Why are the changes needed?

Not truly necessary, I just saw it and fixed it

Does this PR introduce any user-facing change?

No

How was this patch tested?

n/a

Was this patch authored or co-authored using generative AI tooling?

No


cc @dongjoon-hyun


As noted in the PR guidelines...

I state that the contribution is my original work and that I license the work to the project under the project’s open source license.

@github-actions github-actions bot added the CORE label Nov 20, 2023
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Is this the only typo in this test suite, @surjikal ?

@yaooqinn
Copy link
Member

It is not appropriate to use SPARK-34503, which was fixed in v3.2.0.

It would be greatly appreciated if you could modify your title to include [MINOR][TESTS] for minor revisions.

Also, please ensure that there are no spelling, grammar or punctuation errors in EventLogFileWritersSuite.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-34503][CORE][TESTS] Fix EventLogFileWritersSuite typo [MINOR][CORE][TESTS] Fix EventLogFileWritersSuite typo Nov 20, 2023
@dongjoon-hyun
Copy link
Member

Ya, right. We cannot use the released JIRA issue ID. I removed the ID from the PR title according to @yaooqinn 's advice, @surjikal .

@surjikal
Copy link
Contributor Author

Is this the only typo in this test suite, @surjikal ?

No other typos. I just double checked using a spell checker.

Side note: I wasn't intentionally searching for typos btw, I just stumbled upon it.

Ya, right. We cannot use the released JIRA issue ID. I removed the ID from the PR title according to yaooqinn's advice

Noted for next time, thanks for the edit!

@surjikal
Copy link
Contributor Author

surjikal commented Nov 20, 2023

I amended the commit message to match the PR prefix.
I used the issue ID as the branch name too... is this a problem?

@yaooqinn
Copy link
Member

Hi @surjikal, after reviewing the CI https://github.com/apache/spark/pull/43895/checks?check_run_id=18834027975, you need to enable it by following the instructions provided.

I realize I used the issue id as the branch name too... is this a problem?

It's alright, as it doesn't impact the process of reviewing or committing a PR.

@surjikal
Copy link
Contributor Author

surjikal commented Nov 21, 2023

@yaooqinn Done! Apologies, I wrongly assumed I didn't need to do it, given the nature of the change.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @surjikal and all.
Merged to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants