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

Creates PIG Gradle plugin #129

Merged
merged 1 commit into from
Nov 7, 2022
Merged

Creates PIG Gradle plugin #129

merged 1 commit into from
Nov 7, 2022

Conversation

RCHowell
Copy link
Contributor

@RCHowell RCHowell commented Jul 14, 2022

Issue #, if available:

#102
#5

Description of changes:

A Gradle plugin for PIG which generates a task for each type universe in each source set.

Usage

plugins {
    id 'pig-gradle-plugin'
}

sourceSets {
    main {
        pig {
            // in addition to the default 'src/main/pig'
            srcDir 'path/to/type/universes'
        }
    }
    test {
        pig {
            // in addition to the default 'src/test/pig'
            srcDir 'path/to/test/type/universes'
        }
    }
}

pig {
  target = "kotlin" // required
  namespace = ... // optional
  template = ... // optional
  outDir = 'build/generated-sources/pig/<sourceSet>' // default
}

Missing

  • Add dependency on runtime library to project
  • I have a README, but it's not in the root. Should be in the root and wiki.

To Improve
How the plugin dependency is consumed in the pig-tests package.

// It is non-trivial to depend on a local plugin within a gradle project
// The simplest way is using a composite build: https://docs.gradle.org/current/userguide/composite_builds.html
// Other methods involved adding the build/lib/... jar to classpath, or publish to maven local
// By adding the plugin as a dep in `pig-tests`, I cannot use an included build of `pig` in the plugin
// Hence it's much simpler to use the latest published version in the plugin

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Member

@johnedquinn johnedquinn left a comment

Choose a reason for hiding this comment

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

Had a single comment regarding the plugin linking via Gradle. Beyond that, the rest of the PR looks great! Really awesome feature added in a small amount of code.

// Other methods involved adding the build/lib/... jar to classpath, or publish to maven local
// By adding the plugin as a dep in `pig-tests`, I cannot use an included build of `pig` in the plugin
// Hence it's much simpler to use the latest published version in the plugin
implementation("org.partiql:partiql-ir-generator:0.5.0")
Copy link
Member

Choose a reason for hiding this comment

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

We discussed in-person, but just leaving the note here as well for others to discuss. Might be worth looking into alternatives to hard-coding the version. We discussed referencing the JAR if possible. I tried pulling this and going another route but had some issues. Anyways, glad you found a way to make it work, but would be nice to remove the hard-code!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The plugin, runtime, and generator should all be a single gradle project. The issue here is caused by the pig-tests package which, as is, should not be in the main pig gradle project. The pig-tests needs to be fixed to use the generator directly rather than the executable. Executable/plugin usage should be in a separate "pig-example" project which would avoid these oddities. Right now, example usage and testing are intermixed. #130

@RCHowell RCHowell merged commit 83c9f9c into main Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants