-
Notifications
You must be signed in to change notification settings - Fork 34
Create rule S7472: Avoid the usage of unionAll() in favor of union() #4914
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
base: master
Are you sure you want to change the base?
Conversation
|
| "sqKey": "S7472", | ||
| "scope": "All", | ||
| "defaultQualityProfiles": ["Sonar way"], | ||
| "quickfix": "unknown", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I can add a quick fix but I don't know how to do it 😢 Do you know what I should write here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So depending on how we can implement it you can put one of the key word present here
|
joke1196
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good! I have left really small comments about formatting.
rules/S7472/python/rule.adoc
Outdated
| df1 = spark.createDataFrame(data1, ["id", "name"]) | ||
| df2 = spark.createDataFrame(data2, ["id", "name"]) | ||
|
|
||
| # Noncompliant: unionAll() is deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I think it would be best to put it on the same line as the issue raised. This way the diff view contains less changes on SQS.
rules/S7472/python/rule.adoc
Outdated
|
|
||
| == Why is this an issue? | ||
|
|
||
| When using Pyspark, it is recommended to avoid using the `unionAll()` method and instead use the `union()` method when combining two DataFrames. The unionAll() method is deprecated and has been replaced by union(), which provides the same functionality. Using union() ensures compatibility with future versions of PySpark and aligns with current best practices. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sake of consistency, I would maybe put all unionAll() and union() in between backticks. WDYT?
| "quickfix": "unknown", | ||
| "code": { | ||
| "impacts": { | ||
| "MAINTAINABILITY": "MEDIUM", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you have one extra , here...





You can preview this rule here (updated a few minutes after each push).
Review
A dedicated reviewer checked the rule description successfully for: