Skip to content
This repository has been archived by the owner on Nov 11, 2022. It is now read-only.

Fix undefined property bug #634

Merged
merged 2 commits into from
May 7, 2018
Merged

Fix undefined property bug #634

merged 2 commits into from
May 7, 2018

Conversation

chanseokoh
Copy link
Contributor

Fixes #633.

@chanseokoh
Copy link
Contributor Author

I'll fix the test.

@lukecwik lukecwik requested a review from huygaa11 May 3, 2018 17:49
@lukecwik
Copy link
Contributor

lukecwik commented May 3, 2018

@huygaa11 Should this be merged back into Apache Beam?

@chanseokoh
Copy link
Contributor Author

ping

@huygaa11
Copy link
Contributor

huygaa11 commented May 7, 2018

@lukecwik yes, I can do that in a consequent PR.

@huygaa11
Copy link
Contributor

huygaa11 commented May 7, 2018

@chanseokoh If you would like to contribute to Beam, feel free to send a PR to beam repo. Let me know if you are interested :).

Here is the file in Beam:
https://github.com/apache/beam/blob/master/sdks/java/maven-archetypes/starter/src/main/resources/archetype-resources/pom.xml

@chanseokoh
Copy link
Contributor Author

@huygaa11 sure, let me do that.

@chanseokoh
Copy link
Contributor Author

@huygaa11 there's some gradle stuff that I don't understand, so I'll just create a new issue and leave it to you.

@huygaa11
Copy link
Contributor

huygaa11 commented May 7, 2018

No worries. Thanks for looking into it.

@huygaa11 huygaa11 merged commit a071be8 into GoogleCloudPlatform:master May 7, 2018
@chanseokoh
Copy link
Contributor Author

On the Beam repo, I find

build_rules.gradle: maven_exec_plugin: "maven-plugins:maven-exec-plugin:1.6.0",

sdks/java/maven-archetypes/examples/build.gradle: 'maven-exec-plugin.version': dependencies.create(project.library.maven.maven_exec_plugin).getVersion(),

I wonder if they should be exec-maven-plugin in both cases.

Do note that you'll find a lot of instances of maven-exec-plugin with git grep maven-exec-plugin.

@lukecwik
Copy link
Contributor

Sure, swapping so the property name better aligns with the name of the plugin makes sense.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants