Skip to content

[MINOR][SQL] Avoid wildcard import Network in KafkaRDD.#24858

Closed
beliefer wants to merge 1 commit intoapache:masterfrom
beliefer:avoid-wildcard-import
Closed

[MINOR][SQL] Avoid wildcard import Network in KafkaRDD.#24858
beliefer wants to merge 1 commit intoapache:masterfrom
beliefer:avoid-wildcard-import

Conversation

@beliefer
Copy link
Contributor

What changes were proposed in this pull request?

KafkaRDD exists import with wildcard, I think this is a minor task. but it's OK.

How was this patch tested?

Exists UT.

@SparkQA
Copy link

SparkQA commented Jun 13, 2019

Test build #106460 has finished for PR 24858 at commit 4fedcc5.

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

@jerryshao
Copy link
Contributor

There has lots of places using wildcard import, also seems it doesn't violate scalastyle check, so to me it is not so necessary to fix, unless we have a style warning about it.

@HyukjinKwon
Copy link
Member

Yea, let's don't fix this alone.

@beliefer
Copy link
Contributor Author

Yea, let's don't fix this alone.

Do you mean to solve multiple similar problems? or do not need to solve it at all?

@jerryshao
Copy link
Contributor

The changes will introduce lots of conflicts to other PRs, I would suggest to not fix it unless we have a consensus on this style change.

@beliefer
Copy link
Contributor Author

The changes will introduce lots of conflicts to other PRs, I would suggest to not fix it unless we have a consensus on this style change.

Thanks for your review first! I think these conflicts is easy to solve. We have a consensus on this style change is the important thing.

@HyukjinKwon
Copy link
Member

Where was the consensus made? They have been usually rejected.

Since we're going ahead for Spark 3, some changes like this are being merged when they fix multiple cleanups but the changes like this are not encouraged.

@HyukjinKwon
Copy link
Member

e.g, #24857.

Also, causing conflicts itself is a problem because we need to manually fix the conflicts in files. This is an overhead comparing to just git cherry-pick and git push.

Also, think about multiple conflicts like this. Those changes are usually rejected for those reasons because fixing style is virtually nothing real but it causes a real problem.

@beliefer
Copy link
Contributor Author

@jerryshao @HyukjinKwon OK. I get it. I will close this PR.

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.

5 participants