Github user nchammas commented on a diff in the pull request:

    https://github.com/apache/spark/pull/1816#discussion_r15977882
  
    --- Diff: dev/run-tests-jenkins ---
    @@ -24,62 +24,110 @@
     FWDIR="$(cd `dirname $0`/..; pwd)"
     cd "$FWDIR"
     
    +function get_jq () {
    +  # Get jq so we can parse some JSON, man.
    +  # Essential if we want to do anything with the GitHub API responses.
    +  local 
JQ_EXECUTABLE_URL="http://stedolan.github.io/jq/download/linux64/jq";
    +
    +  echo "Fetching jq from ${JQ_EXECUTABLE_URL}"
    +  curl --silent --output "$FWDIR/dev/jq" "$JQ_EXECUTABLE_URL"
    +  local curl_status=$?
    +
    +  if [ $curl_status -ne 0 ]; then
    +      echo "Failed to get jq." >&2
    +      return $curl_status
    +  fi
    +
    +  chmod u+x "$FWDIR/dev/jq"
    +}
    +
     
COMMENTS_URL="https://api.github.com/repos/apache/spark/issues/$ghprbPullId/comments";
     
    -function post_message {
    -  message=$1
    -  data="{\"body\": \"$message\"}"
    +function post_message () {
    +  local message=$1
    +  local data="{\"body\": \"$message\"}"
       echo "Attempting to post to Github:"
       echo "$data"
    -
    -  curl -D- -u x-oauth-basic:$GITHUB_OAUTH_KEY -X POST --data "$data" -H \
    -    "Content-Type: application/json" \
    -    $COMMENTS_URL | head -n 8
    +  
    +  # comment_id=$(
    +  curl `#--dump-header -` \
    +    --silent \
    +    --user x-oauth-basic:$GITHUB_OAUTH_KEY \
    +    --request POST \
    +    --data-urlencode ="$data" `# for make benefit @, &, and other 
annoyances` \
    +    --write-out "HTTP Response: %{http_code}" \
    +    --header "Content-Type: application/json" \
    +    "$COMMENTS_URL" #> /dev/null #| "$FWDIR/dev/jq" .id #| head -n 8
    +  # )
    +  local curl_status=${PIPESTATUS[0]}
    +  
    +  if [ $curl_status -ne 0 ]; then
    +      echo "Failed to post message to GitHub." >&2
    +      exit $curl_status
    +  fi
     }
     
    -start_message="QA tests have started for PR $ghprbPullId."
    -if [ "$sha1" == "$ghprbActualCommit" ]; then
    -  start_message="$start_message This patch DID NOT merge cleanly! "
    -else
    -  start_message="$start_message This patch merges cleanly. "
    -fi
    -start_message="$start_message<br>View progress: "
    -start_message="$start_message${BUILD_URL}consoleFull"
    +commit_url="https://github.com/apache/spark/commit/${ghprbActualCommit}";
    +# GitHub doesn't auto-links short hashes when submitted via the API, 
unfortunately. :(
    +short_commit_hash=${ghprbActualCommit:0:7}
    +
    +start_message="\
    +[QA tests have started](${BUILD_URL}consoleFull) for \
    +PR $ghprbPullId at commit 
[\`${short_commit_hash}\`](${short_commit_hash}).\n"
    +
    +# merge status is built in to GitHub; do we need it here?
    --- End diff --
    
    > sometimes people don't realize their patch wasn't mergeable, it goes 
through all the tests, and then they have to up-merge it and re-run the tests.
    
    I see. Do you think the built-in GitHub banner is not enough to make this 
clear? I think adding in little things that save contributors' time as you 
describe is a good thing, so I'll add it back in if you think it will make a 
difference.


---
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

Reply via email to