[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-09 Thread pwendell
Github user pwendell commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62313063
  
Okay i'm gonna close this. If one of you guys could quickly add docs on our 
wiki, that would be great.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-09 Thread pwendell
Github user pwendell closed the pull request at:

https://github.com/apache/spark/pull/3165


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread JoshRosen
Github user JoshRosen commented on a diff in the pull request:

https://github.com/apache/spark/pull/3165#discussion_r20050583
  
--- Diff: dev/fetch-pr ---
@@ -0,0 +1,53 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the License); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an AS IS BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# A utility script for fetching local copies of a pull request.
+
+set -e
+set -o pipefail
+
+usage(){
+  echo Utility for fetching a specific pull request into a temporary ref.
+  echo Usage: ./dev/fetch-pr pr-num
+  exit 1
+}
+
+case $1 in
+  ) usage ;;
+  -h) usage ;;
+  h) usage ;;
+  --help) usage ;;
+  -help) usage ;;
+  help) usage ;;
+esac
+
+pr_num=$1
+# Find remote corresponding to github
+remote_name=$(git remote -v |grep apache/spark\.git | grep fetch | \
--- End diff --

Tossing a `head -n1` into this pipeline solves this problem, though.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62249644
  
This looks good to me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread nchammas
Github user nchammas commented on a diff in the pull request:

https://github.com/apache/spark/pull/3165#discussion_r20050584
  
--- Diff: dev/fetch-pr ---
@@ -0,0 +1,53 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the License); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an AS IS BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# A utility script for fetching local copies of a pull request.
+
+set -e
+set -o pipefail
+
+usage(){
+  echo Utility for fetching a specific pull request into a temporary ref.
+  echo Usage: ./dev/fetch-pr pr-num
+  exit 1
+}
+
+case $1 in
+  ) usage ;;
+  -h) usage ;;
+  h) usage ;;
+  --help) usage ;;
+  -help) usage ;;
+  help) usage ;;
+esac
+
+pr_num=$1
+# Find remote corresponding to github
+remote_name=$(git remote -v |grep apache/spark\.git | grep fetch | \
+  awk '{ print $1; }')
--- End diff --

Some minor style comments:

You can format this so that each command in the pipeline is on a separate 
line.

```shell
remote_name=$(
  git remote -v \
  | grep apache/spark\.git \
  | grep (fetch)$ \
  | awk '{ print $1; }'
)
```

May I also suggest quoting the search terms and restricting the search for 
`fetch` to lines ending in the word.

Finally, if there is any chance this filter might return more than one 
result, we should probably add `-m 1` to the first grep statement.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread nchammas
Github user nchammas commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62249680
  
Whoops, I missed Josh's comments.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62249778
  
Yeah, it looks like there was a race condition.  Good style point about 
separating the pipeline commands onto their own lines; I always overlook these 
kind of bash readability tricks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62251084
  
  [Test build #23092 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23092/consoleFull)
 for   PR 3165 at commit 
[`aacb814`](https://github.com/apache/spark/commit/aacb814cf8c785298d03389624913be376f79fb5).
 * This patch **passes all tests**.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62251086
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23092/
Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62279046
  
Why not using hub? It is super easy and we don't need to duplicate the 
work. 

https://github.com/github/hub


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread rxin
Github user rxin commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62279935
  
Just realized Josh also pointed this out earlier. I've been using hub and 
it's pretty good. It does require a local branch, but that's pretty easy to do.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread pwendell
Github user pwendell commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62288892
  
I didn't use hub - but I preferred this since I can just create a temporary 
ref that gets thrown away and I never have to worry about cleaning it up.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-08 Thread pwendell
Github user pwendell commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62288954
  
I'm actually happy to just drop this though if we can update the 
documentation in our wiki to suggest people use hub. @JoshRosen or @rxin would 
one of you guys be able to put a few lines in 
https://cwiki.apache.org/confluence/display/SPARK/Useful+Developer+Tools with 
the process you use? I can then verify and if it's all good I can just drip 
this PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-07 Thread pwendell
GitHub user pwendell opened a pull request:

https://github.com/apache/spark/pull/3165

SPARK-3648: Provide a script for fetching remote PR's for review

This is a simple script I wrote for fetching remote pull requests.
I found this very useful during code reviews and I think other
developers would as well.

One nice thing is this allows you to avoid creating local refs for
all pull requests, which can be annoying since there is high churn
in PR activity.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/pwendell/spark fetch-pr

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/3165.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #3165


commit aacb814cf8c785298d03389624913be376f79fb5
Author: Patrick Wendell pwend...@gmail.com
Date:   2014-11-08T07:23:37Z

SPARK-3648: Provide a script for fetching remote PR's for review

This is a simple script I wrote for fetching remote pull requests.
I found this very useful during code reviews and I think other
developers would as well.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-07 Thread pwendell
Github user pwendell commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62248992
  
@JoshRosen mind taking a quick look?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-07 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62249112
  
  [Test build #23092 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23092/consoleFull)
 for   PR 3165 at commit 
[`aacb814`](https://github.com/apache/spark/commit/aacb814cf8c785298d03389624913be376f79fb5).
 * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-07 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62249465
  
In order to try out this PR's script, I used `hub checkout 
https://github.com/apache/spark/pull/3165` to fetch this PR, which created a 
new branch named `pwendell-fetch-pr`.  Do you think that we still need this if 
[`hub`](https://github.com/github/hub) exists?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-07 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/3165#issuecomment-62249570
  
On the other hand, I guess that `hub` pollutes your local git branch 
namespace by creating local tracking branches for each PR that you checkout, 
which could quickly get annoying at the scale that we've been working at.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: SPARK-3648: Provide a script for fetching remo...

2014-11-07 Thread JoshRosen
Github user JoshRosen commented on a diff in the pull request:

https://github.com/apache/spark/pull/3165#discussion_r20050579
  
--- Diff: dev/fetch-pr ---
@@ -0,0 +1,53 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the License); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an AS IS BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# A utility script for fetching local copies of a pull request.
+
+set -e
+set -o pipefail
+
+usage(){
+  echo Utility for fetching a specific pull request into a temporary ref.
+  echo Usage: ./dev/fetch-pr pr-num
+  exit 1
+}
+
+case $1 in
+  ) usage ;;
+  -h) usage ;;
+  h) usage ;;
+  --help) usage ;;
+  -help) usage ;;
+  help) usage ;;
+esac
+
+pr_num=$1
+# Find remote corresponding to github
+remote_name=$(git remote -v |grep apache/spark\.git | grep fetch | \
--- End diff --

It turns out that I have multiple remotes that point to the Apache Spark 
Github:

```
git remote -v |grep apache/spark\.git | grep fetch
apache-github   git://github.com/apache/spark.git (fetch)
origin  g...@github.com:apache/spark.git (fetch)
```

This caused the checkout to fail:

```
./dev/fetch-pr 3000
Detected Apache github remote 'apache-github
origin'
Fetching remote ref 'pull/3000/head'
fatal: Couldn't find remote ref origin
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org