Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

What changes were proposed in this pull request?

This PR merges multiple lines enumerating items in order to remove the redundant spaces following slashes in Structured Streaming Programming Guide in 2.0.2-rc1.

  • Before: Scala/ Java/ Python
  • After: Scala/Java/Python

How was this patch tested?

Manual by the followings because this is documentation update.

cd docs
SKIP_API=1 jekyll build

@SparkQA
Copy link

SparkQA commented Oct 30, 2016

Test build #67782 has finished for PR 15686 at commit b385a4c.

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

@dongjoon-hyun
Copy link
Member Author

Thank you for review and approval, @srowen !

@HyukjinKwon
Copy link
Member

Build started: [SparkR] ALL PR-15686
Diff: master...spark-test:1D4EC6E8-F2CF-4585-9745-0AE5956F211C

:)

@rxin
Copy link
Contributor

rxin commented Oct 31, 2016

@HyukjinKwon why are the appveyor checks failing?

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Oct 31, 2016

@rxin, it seems spurious. The message seems meaning the failure when the commit is virtually not mergeable[1].

It seems it fails time to time for various reasons. For example, in some cases, it seems failing to clone due to network problem within AppVeyor[2].

it is able to re-trigger via Web UI but it seems committers are also not allowed to access to it. Due to this problem, I temporarily made a bunch of scripts[3] to manually launch another build via @spark-test account. I can run the build and leave the comment above until we find a better way to handle it. (treat me like a bot)

[1]http://help.appveyor.com/discussions/problems/4912-merge-commit-does-not-start-build-in-the-repo-i-make-pull-request-for
[2]https://ci.appveyor.com/project/ApacheSoftwareFoundation/spark/build/256-master
[3]https://github.com/HyukjinKwon/spark-appveyor

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Oct 31, 2016

I believe the message indicates the same case with the PR 15673 and 15689 but it seems spurious in this case.

@srowen
Copy link
Member

srowen commented Oct 31, 2016

This change seems fine.

If Appveyor is encountering errors regularly, we might consider disabling it, just because it wouldn't be giving useful information if most failures were spurious.

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Oct 31, 2016

There are three problems with it.

  • it starts the build when someone merges the latest upstream (not rebases) when pushing more commits in its PR (as the merged one usually has the changes in R) - this case seems fine though because usually it succeeds to build. Also actually I asked this case to AppVeyor but I haven't received the reply.
  • it sometimes fails to build (it is relatively rare I believe).
  • it leaves a failure mark on other branches when someone opens a PR from a branch to another branch.

I am neutral because it seems the cases are not often. I may open a JIRA if I incline toward disabling this or please feel free to create a JIRA if anyone feels so. (Let me please cc @shivaram. I would like him to know about this).

@shivaram
Copy link
Contributor

shivaram commented Oct 31, 2016

Yeah I think it'll be a good idea to know the error rate we hit. @HyukjinKwon It might also be good to create an INFRA ticket to see if we can re-trigger the AppVeyor builds somehow ?

@HyukjinKwon
Copy link
Member

@dongjoon-hyun I am sorry for unrelated comments here. All these comments are not related with this PR.

@shivaram Sure, Let me try to create a JIRA. I will cc you. We might be able to talk more there.

@dongjoon-hyun
Copy link
Member Author

Never mind, @hyunjinkwon .
I was also curious about AppVoyer failure. :)

@srowen
Copy link
Member

srowen commented Nov 1, 2016

Merged to master

@asfgit asfgit closed this in 623fc7f Nov 1, 2016
@dongjoon-hyun
Copy link
Member Author

Thank you for merging, @srowen .

@dongjoon-hyun dongjoon-hyun deleted the minor_doc_space branch November 7, 2016 00:48
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
## What changes were proposed in this pull request?

This PR merges multiple lines enumerating items in order to remove the redundant spaces following slashes in [Structured Streaming Programming Guide in 2.0.2-rc1](http://people.apache.org/~pwendell/spark-releases/spark-2.0.2-rc1-docs/structured-streaming-programming-guide.html).
- Before: `Scala/ Java/ Python`
- After: `Scala/Java/Python`
## How was this patch tested?

Manual by the followings because this is documentation update.

```
cd docs
SKIP_API=1 jekyll build
```

Author: Dongjoon Hyun <dongjoon@apache.org>

Closes apache#15686 from dongjoon-hyun/minor_doc_space.
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