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

[MINOR] Fix typos in dev/* scripts. #20436

Closed
wants to merge 3 commits into from

Conversation

ashashwat
Copy link
Contributor

What changes were proposed in this pull request?

Consistency in style, grammar and removal of extraneous characters.

How was this patch tested?

Manually as this is a doc change.

@@ -60,9 +60,9 @@ export "PYLINT_HOME=$PYTHONPATH"
export "PATH=$PYTHONPATH:$PATH"

# There is no need to write this output to a file
#+ first, but we do so so that the check status can
#+ be output before the report, like with the
#+ scalastyle and RAT checks.
Copy link
Member

Choose a reason for hiding this comment

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

Ah, this wasn't from SPARK-23174 tho.

Copy link
Member

Choose a reason for hiding this comment

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

Hmmm .. seems this indicates line continuation and intentional .. ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that so? We have 100 character limit on a single line according to the style guide. Maybe all 4 lines could be rearranged?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm .. @nchammas, some of these seem added by you a long ago. Could you take a look for this one please? I think I am not aware of this. Wonder if we can just remove those.

Copy link
Contributor

Choose a reason for hiding this comment

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

The #+ convention is something I picked up from the Linux Documentation Project, if that's what you're referring to. You can safely do away with it and just have the #.

It was a "phase". I'm over it now... 😆

Copy link
Member

Choose a reason for hiding this comment

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

Wow, thank you so much.

@HyukjinKwon
Copy link
Member

ok to test

@HyukjinKwon
Copy link
Member

I would rather suggest to fix these together in the followup already open, 20432 or find out more typos while we are here.
Can you take another look for other scripts and check if there are similar instances or typos? It'd be nicer if we fix them in batch.

@ashashwat
Copy link
Contributor Author

@HyukjinKwon Let me go ahead and check all the scripts for similar instances or typos.

@HyukjinKwon
Copy link
Member

@ashashwat, thanks. Let's update the PR title to something like .. [MINOR] Fix typos in dev/* scripts.

@ashashwat ashashwat changed the title [SPARK-23174][DOC][PYTHON] python code style checker update fix. [MINOR] Fix typos in dev/* scripts. Jan 30, 2018
@SparkQA
Copy link

SparkQA commented Jan 30, 2018

Test build #86816 has finished for PR 20436 at commit 78cb070.

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

@SparkQA
Copy link

SparkQA commented Jan 30, 2018

Test build #86821 has finished for PR 20436 at commit d2d157d.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 30, 2018

Test build #86828 has finished for PR 20436 at commit 0a09dcb.

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

@HyukjinKwon
Copy link
Member

Merged to master.

@asfgit asfgit closed this in 9623a98 Jan 30, 2018
@ashashwat ashashwat deleted the SPARK-23174 branch January 30, 2018 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants