Skip to content

Simplify build files for pipelines module #51089

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

Closed
wants to merge 32 commits into from
Closed

Conversation

sryza
Copy link
Contributor

@sryza sryza commented Jun 4, 2025

What changes were proposed in this pull request?

This is a followup PR to #51003, which removes extraneous elements from the sbt and Maven build files, as suggested by @LuciferYang.

Why are the changes needed?

Does this PR introduce any user-facing change?

No

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

No

</dependencies>
<build>
<outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
<testOutputDirectory>target/scala-${scala.binary.version}/test-classes</testOutputDirectory>
<plugins>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, this plugin is also redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to simplify – I just took this out.

@@ -884,6 +886,36 @@ object SparkConnectClient {
)
}

object SparkDeclarativePipelines {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far, it seems that SparkDeclarativePipelines doesn't require any additional configuration. I changed it to

object SparkDeclarativePipelines {
  lazy val settings = Seq()
}

and then executed build/sbt clean "pipelines/test". All the tests still passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great - I just took this out.

@sryza
Copy link
Contributor Author

sryza commented Jun 5, 2025

I ended up including this inside the underlying PR, so now closing

@sryza sryza closed this Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants