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

[SPARK-46596][CORE][TESTS] Correct package name of SslTestUtils #44596

Closed

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Jan 4, 2024

What changes were proposed in this pull request?

The package name for SslTestUtils is defined as org.apache.spark, but it is in the org/apache/spark/util/ directory. This pr corrects its package name to ensure consistency.

Why are the changes needed?

Correct package name of SslTestUtils

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GitHub Actions

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

No

@github-actions github-actions bot added the CORE label Jan 4, 2024
@@ -15,8 +15,9 @@
* limitations under the License.
*/

package org.apache.spark
package org.apache.spark.util
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @dongjoon-hyun FYI

Moving this file to the core/src/test/scala/org/apache/spark/ directory is a solution that involves fewer changes. If you prefer to move the file instead of fixing the package name, please let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like an incorrect package name - keeping it in util makes more sense.
Thanks for fixing this @LuciferYang !

@MaxGekk
Copy link
Member

MaxGekk commented Jan 4, 2024

The failure https://github.com/LuciferYang/spark/actions/runs/7409548578/job/20163613930 is not related to the changes, and I think it has been fixed by #44594. Let me merge this.

@MaxGekk
Copy link
Member

MaxGekk commented Jan 4, 2024

+1, LGTM. Merging to master.
Thank you, @LuciferYang and @mridulm for review.

@MaxGekk MaxGekk closed this in ddcfe6b Jan 4, 2024
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.

@LuciferYang
Copy link
Contributor Author

Thanks @MaxGekk @mridulm and @dongjoon-hyun ~

@LuciferYang LuciferYang deleted the SslTestUtils-package-name branch January 5, 2024 06:02
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.

4 participants