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

[SPARK-36712][BUILD][FOLLOWUP] Improve the regex to avoid breaking pom.xml #33996

Closed
wants to merge 1 commit into from
Closed

[SPARK-36712][BUILD][FOLLOWUP] Improve the regex to avoid breaking pom.xml #33996

wants to merge 1 commit into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Sep 14, 2021

What changes were proposed in this pull request?

This PR aims to fix the regex to avoid breaking pom.xml.

Why are the changes needed?

BEFORE

$ dev/change-scala-version.sh 2.12
$ git diff | head -n10
diff --git a/core/pom.xml b/core/pom.xml
index dbde22f2bf..6ed368353b 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -35,7 +35,7 @@
   </properties>

   <dependencies>
-    <!-- #if scala-2.13 --><!--
+    <!-- #if scala-2.13 --><!--<!--

AFTER
Since the default Scala version is 2.12, the following no-op is the correct behavior which is consistent with the previous behavior.

$ dev/change-scala-version.sh 2.12
$ git diff

Does this PR introduce any user-facing change?

No. This is a dev only change.

How was this patch tested?

Manually.

@dongjoon-hyun
Copy link
Member Author

cc @lrytz

@srowen
Copy link
Member

srowen commented Sep 14, 2021

Oh was it not working to go back to 2.12? ah OK in any event, good catch.
This is a hacky hack for sure, which eventually goes away with 2.12 support

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Sep 14, 2021

Yes, this PR restores the previous behavior. It was no-op when we use the same scala version before SPARK-36712.

Although we switched to Scala 2.13 by default, SPARK-36712's regex will fail dev/change-scala-version.sh 2.13 due to the same reason.

@dongjoon-hyun
Copy link
Member Author

cc @sunchao and @viirya

@SparkQA
Copy link

SparkQA commented Sep 14, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47783/

@SparkQA
Copy link

SparkQA commented Sep 14, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47783/

@dongjoon-hyun
Copy link
Member Author

Thank you, @srowen , @viirya , @sunchao .
Merged to master/3.2.

dongjoon-hyun added a commit that referenced this pull request Sep 14, 2021
…m.xml

### What changes were proposed in this pull request?

This PR aims to fix the regex to avoid breaking `pom.xml`.

### Why are the changes needed?

**BEFORE**
```
$ dev/change-scala-version.sh 2.12
$ git diff | head -n10
diff --git a/core/pom.xml b/core/pom.xml
index dbde22f2bf..6ed368353b 100644
--- a/core/pom.xml
+++ b/core/pom.xml
 -35,7 +35,7
   </properties>

   <dependencies>
-    <!--<!--
```

**AFTER**
Since the default Scala version is `2.12`, the following `no-op` is the correct behavior which is consistent with the previous behavior.
```
$ dev/change-scala-version.sh 2.12
$ git diff
```

### Does this PR introduce _any_ user-facing change?

No. This is a dev only change.

### How was this patch tested?

Manually.

Closes #33996 from dongjoon-hyun/SPARK-36712.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit d730ef2)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@SparkQA
Copy link

SparkQA commented Sep 14, 2021

Test build #143277 has finished for PR 33996 at commit 676991b.

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

@dongjoon-hyun dongjoon-hyun deleted the SPARK-36712 branch September 15, 2021 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants