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

[SPARK-22082][SparkR]Spelling mistake: "choosen" in API doc of R. #19300

Closed
wants to merge 1 commit into from

Conversation

zuotingbing
Copy link

What changes were proposed in this pull request?

"choosen" should be "chosen" in API doc of R.
http://spark.apache.org/docs/latest/api/R/index.html , see spark.kmeans

How was this patch tested?

NA

@zuotingbing zuotingbing changed the title [SPARK-22082][SparkR]Spelling mistake: choosen in API doc of R. [SPARK-22082][SparkR]Spelling mistake: "choosen" in API doc of R. Sep 21, 2017
@srowen
Copy link
Member

srowen commented Sep 21, 2017

Please review and fix typos in a whole batch of code, or don't bother and close this

@HyukjinKwon
Copy link
Member

I am fixing some codes around here - https://github.com/apache/spark/pull/19290/files#diff-d9f92e07db6424e2527a7f9d7caa9013R328. If this one is only the one, let me fold this into mine.

@zuotingbing
Copy link
Author

or don't bother and close this

I have not found other typos and sorry i'm not agree with you. Since this spelling mistake affects the API doc in official website, we should fix it of course rather than 'don't bother and close this'. Thanks.

@HyukjinKwon
Copy link
Member

I don't think we should ever encourage to fix a single typo. It doesn't look making any harm to the context or contents really.

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Sep 21, 2017

I can give you many reasons - reviewing cost, build cost (including retriggering Jenkins build when failed unexpectedly, for example, -9 signal), potentially making conflicts / rebasing in other PRs, potentially swarming PRs and potentially inactive PRs that someone should ping (and should be manually closed by another PR). Do you really think it is worth fixing it over those minimal costs?

@zuotingbing
Copy link
Author

@cloud-fan @gatorsmile Could you please help to review this and merge it to master? Thanks.

@srowen
Copy link
Member

srowen commented Sep 22, 2017

@HyukjinKwon will make this change in another PR. We do need to push back on tiny PRs. It's a problem because some people are actually incentivized based on number of patches and it costs the project to accept that.

@zuotingbing
Copy link
Author

if we find some mistakes likes this , shouldn't we to report it at first?

@HyukjinKwon
Copy link
Member

I'd help review other PRs and leave a comment when someone fixes some codes around it rather than proposing it alone, or you can buffer it in your local and flush it.

@felixcheung
Copy link
Member

or find and fix more typos ;)

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.

4 participants