-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-29936][R] Fix SparkR lint errors and add lint-r GitHub Action #26564
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
Conversation
This comment has been minimized.
This comment has been minimized.
|
Only one instance is left. It's a Window-specific function. |
This comment has been minimized.
This comment has been minimized.
R/pkg/.lintr
Outdated
| @@ -1,2 +1,2 @@ | |||
| linters: with_defaults(line_length_linter(100), multiple_dots_linter = NULL, object_name_linter = NULL, camel_case_linter = NULL, open_curly_linter(allow_single_line = TRUE), closed_curly_linter(allow_single_line = TRUE)) | |||
| linters: with_defaults(line_length_linter(100), multiple_dots_linter = NULL, object_name_linter = NULL, camel_case_linter = NULL, open_curly_linter(allow_single_line = TRUE), closed_curly_linter(allow_single_line = TRUE), object_usage_linter = NULL) | |||
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.
object_usage_linter has a limitation which cannot detect shell function used in Windows environment. Also, nolint doesn't work for this rule. This is a known limitation of lint-r itself.
|
cc @srowen , @HyukjinKwon , @viirya , @felixcheung |
|
Also, cc @dbtsai |
This comment has been minimized.
This comment has been minimized.
|
Test build #113973 has finished for PR 26564 at commit
|
|
Now, this PR passes All tests ( |
|
Could you review this PR, @HyukjinKwon ? |
|
Test build #113980 has finished for PR 26564 at commit
|
HyukjinKwon
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.
LGTM if both tests pass
|
Unfortunately, |
|
For the Jenkins, |
|
Thank you, @HyukjinKwon and @viirya ! This PR become much better than before. |
|
Test build #113987 has finished for PR 26564 at commit
|
|
Thanks!
|
What changes were proposed in this pull request?
This PR fixes SparkR lint errors and adds
lint-rGitHub Action to protect the branch.Why are the changes needed?
It turns out that we currently don't run it. It's recovered yesterday. However, after that, our Jenkins linter jobs (
master/branch-2.4) has been broken onlint-rtasks.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass the GitHub Action on this PR in addition to Jenkins R and AppVeyor R.