-
Notifications
You must be signed in to change notification settings - Fork 28.6k
[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
|
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-r
GitHub 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-r
tasks.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.