You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@bahir.apache.org by lukasz-antoniak <gi...@git.apache.org> on 2018/12/11 15:00:08 UTC

[GitHub] bahir pull request #76: [BAHIR-107] Upgrade to Scala 2.12 and Spark 2.4.0

GitHub user lukasz-antoniak opened a pull request:

    https://github.com/apache/bahir/pull/76

    [BAHIR-107] Upgrade to Scala 2.12 and Spark 2.4.0

    JIRA ticket: https://issues.apache.org/jira/browse/BAHIR-107.

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

    $ git pull https://github.com/lukasz-antoniak/bahir BAHIR-107

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

    https://github.com/apache/bahir/pull/76.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 #76
    
----
commit d1ee3be099ec11869d83f5687c8a1b4822f0a728
Author: Lukasz Antoniak <lu...@...>
Date:   2018-12-11T14:57:46Z

    [BAHIR-107] Upgrade to Scala 2.12 and Spark 2.4.0

----


---

[GitHub] bahir pull request #76: [BAHIR-107] Upgrade to Scala 2.12 and Spark 2.4.0

Posted by lukasz-antoniak <gi...@git.apache.org>.
Github user lukasz-antoniak commented on a diff in the pull request:

    https://github.com/apache/bahir/pull/76#discussion_r250347037
  
    --- Diff: dev/release-build.sh ---
    @@ -231,83 +229,92 @@ function checkout_code {
         git_hash=`git rev-parse --short HEAD`
         echo "Checked out Bahir git hash $git_hash"
     
    -    cd "$BASE_DIR" #return to base dir
    +    cd "$BASE_DIR" # return to base dir
     }
     
     if [[ "$RELEASE_PREPARE" == "true" ]]; then
         echo "Preparing release $RELEASE_VERSION"
    -    # Checkout code
    +    # checkout code
         checkout_code
         cd target/bahir
     
    -    # Build and prepare the release
    -    $MVN $PUBLISH_PROFILES release:clean release:prepare $DRY_RUN -Darguments="-Dgpg.passphrase=\"$GPG_PASSPHRASE\" -DskipTests" -DreleaseVersion="$RELEASE_VERSION" -DdevelopmentVersion="$DEVELOPMENT_VERSION" -Dtag="$RELEASE_TAG"
    +    # test with scala 2.11 and 2.12
    +    ./dev/change-scala-version.sh 2.11
    +    $MVN $PUBLISH_PROFILES clean test -Dscala-2.11 || exit 1
    +    ./dev/change-scala-version.sh 2.12
    +    $MVN $PUBLISH_PROFILES clean test || exit 1
    +
    +    # build and prepare the release
    +    $MVN $PUBLISH_PROFILES release:clean release:prepare $DRY_RUN \
    +        -DskipTests=true -Dgpg.passphrase="$GPG_PASSPHRASE" \
    +        -DreleaseVersion="$RELEASE_VERSION" -DdevelopmentVersion="$DEVELOPMENT_VERSION" -Dtag="$RELEASE_TAG"
     
    -    cd .. #exit bahir
    +    cd .. # exit bahir
     
         if [ -z "$DRY_RUN" ]; then
             cd "$BASE_DIR/target/bahir"
             git checkout $RELEASE_TAG
             git clean -d -f -x
     
    -        $MVN $PUBLISH_PROFILES clean install -DskiptTests -Darguments="-DskipTests"
    +        $MVN $PUBLISH_PROFILES clean install -DskipTests=true
     
    --- End diff --
    
    `-DskipTests` does not require arguments, but Scala tests do not honour it, see: https://github.com/scalatest/scalatest/issues/466. On my machine, while running commands from original version of release script, tests are were being executed.


---

[GitHub] bahir pull request #76: [BAHIR-107] Upgrade to Scala 2.12 and Spark 2.4.0

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/bahir/pull/76


---

[GitHub] bahir issue #76: [BAHIR-107] Upgrade to Scala 2.12 and Spark 2.4.0

Posted by lresende <gi...@git.apache.org>.
Github user lresende commented on the issue:

    https://github.com/apache/bahir/pull/76
  
    Could you start a conversation on the dev list regarding the direction this is going to take, are we droping 2.10 and continue support with 211 and 212 but making 212 the default, and all other details... at least to be informative to the dev community


---

[GitHub] bahir issue #76: [BAHIR-107] Upgrade to Scala 2.12 and Spark 2.4.0

Posted by lukasz-antoniak <gi...@git.apache.org>.
Github user lukasz-antoniak commented on the issue:

    https://github.com/apache/bahir/pull/76
  
    Rebased with latest changes form master branch. I have also fixed `skipTests` handling in the release script (it was not working before) and added another stage to actually execute unit tests with Scala 2.11 and 2.12 before publishing. Can be easily removed if you disagree.


---

[GitHub] bahir pull request #76: [BAHIR-107] Upgrade to Scala 2.12 and Spark 2.4.0

Posted by lresende <gi...@git.apache.org>.
Github user lresende commented on a diff in the pull request:

    https://github.com/apache/bahir/pull/76#discussion_r250337448
  
    --- Diff: dev/release-build.sh ---
    @@ -231,83 +229,92 @@ function checkout_code {
         git_hash=`git rev-parse --short HEAD`
         echo "Checked out Bahir git hash $git_hash"
     
    -    cd "$BASE_DIR" #return to base dir
    +    cd "$BASE_DIR" # return to base dir
     }
     
     if [[ "$RELEASE_PREPARE" == "true" ]]; then
         echo "Preparing release $RELEASE_VERSION"
    -    # Checkout code
    +    # checkout code
         checkout_code
         cd target/bahir
     
    -    # Build and prepare the release
    -    $MVN $PUBLISH_PROFILES release:clean release:prepare $DRY_RUN -Darguments="-Dgpg.passphrase=\"$GPG_PASSPHRASE\" -DskipTests" -DreleaseVersion="$RELEASE_VERSION" -DdevelopmentVersion="$DEVELOPMENT_VERSION" -Dtag="$RELEASE_TAG"
    +    # test with scala 2.11 and 2.12
    +    ./dev/change-scala-version.sh 2.11
    +    $MVN $PUBLISH_PROFILES clean test -Dscala-2.11 || exit 1
    +    ./dev/change-scala-version.sh 2.12
    +    $MVN $PUBLISH_PROFILES clean test || exit 1
    +
    +    # build and prepare the release
    +    $MVN $PUBLISH_PROFILES release:clean release:prepare $DRY_RUN \
    +        -DskipTests=true -Dgpg.passphrase="$GPG_PASSPHRASE" \
    +        -DreleaseVersion="$RELEASE_VERSION" -DdevelopmentVersion="$DEVELOPMENT_VERSION" -Dtag="$RELEASE_TAG"
     
    -    cd .. #exit bahir
    +    cd .. # exit bahir
     
         if [ -z "$DRY_RUN" ]; then
             cd "$BASE_DIR/target/bahir"
             git checkout $RELEASE_TAG
             git clean -d -f -x
     
    -        $MVN $PUBLISH_PROFILES clean install -DskiptTests -Darguments="-DskipTests"
    +        $MVN $PUBLISH_PROFILES clean install -DskipTests=true
     
    --- End diff --
    
    My understanding is that -DskiptTests does not require a value as described in [Maven docs](http://maven.apache.org/surefire/maven-surefire-plugin/examples/skipping-tests.html).
    
    Also, -arguments is required to pass arguments to internal plugins such as the release plugin.


---