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 import order and other coding style #3966

Closed
wants to merge 17 commits into from

Conversation

Bilna
Copy link

@Bilna Bilna commented Jan 9, 2015

fixed import order and other coding style

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@rxin
Copy link
Contributor

rxin commented Jan 9, 2015

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Jan 9, 2015

Test build #25304 has started for PR 3966 at commit 5e76f04.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 9, 2015

Test build #25304 has finished for PR 3966 at commit 5e76f04.

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

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25304/
Test FAILed.

@srowen
Copy link
Member

srowen commented Jan 9, 2015

I think the stance has been to avoid one-off non-functional style changes for their own sake, as it buys almost nothing and costs time to review/test/resolve merge conflicts.

If this were going to proceed, I would say you should also squash the ~20 commits here to one.

@rxin
Copy link
Contributor

rxin commented Jan 9, 2015

@srowen this one might be ok since it is limited to a smaller scope - but I agree in general it is not a good idea to do massive style changes. When we merge, the merge script does automatic squash, so that it is not a huge problem here (but still would be nice if pr owners can squash for clarity in general).

@andrewor14
Copy link
Contributor

Ok, this one doesn't seem too invasive so I will go ahead and merge it in master. Thanks.

@asfgit asfgit closed this in 4e1f12d Jan 9, 2015
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