-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fixes to Actions and Transformers #15
base: develop
Are you sure you want to change the base?
Conversation
src/main/java/io/cdap/plugin/cloud/vision/action/ActionConstants.java
Outdated
Show resolved
Hide resolved
src/main/java/io/cdap/plugin/cloud/vision/action/package-info.java
Outdated
Show resolved
Hide resolved
src/main/java/io/cdap/plugin/cloud/vision/exception/package-info.java
Outdated
Show resolved
Hide resolved
src/main/java/io/cdap/plugin/cloud/vision/transform/ExtractorTransformConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/io/cdap/plugin/cloud/vision/transform/ExtractorTransformConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/io/cdap/plugin/cloud/vision/transform/document/package-info.java
Outdated
Show resolved
Hide resolved
src/main/java/io/cdap/plugin/cloud/vision/transform/image/package-info.java
Outdated
Show resolved
Hide resolved
src/main/java/io/cdap/plugin/cloud/vision/transform/package-info.java
Outdated
Show resolved
Hide resolved
src/main/java/io/cdap/plugin/cloud/vision/transform/schema/package-info.java
Outdated
Show resolved
Hide resolved
src/test/java/io/cdap/plugin/cloud/vision/GcsBucketHelperTest.java
Outdated
Show resolved
Hide resolved
src/test/java/io/cdap/plugin/cloud/vision/action/OfflineImageExtractorActionTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pbo-cirus made some initial comments. Thanks a lot for fixing the indentation changes for the most part. Could you also please remove the package-info.java files - we generally don't add them in plugin code.
Ok, I'll work on it tomorrow.
Thanks.
…On Thu, Apr 9, 2020, 9:18 PM Bhooshan Mogal ***@***.***> wrote:
***@***.**** requested changes on this pull request.
@pbo-cirus <https://github.com/pbo-cirus> made some initial comments.
Thanks a lot for fixing the indentation changes for the most part. Could
you also please remove the package-info.java files - we generally don't add
them in plugin code.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/APDAA6HU6IYJOXTYFA4TBUTRLZXWLANCNFSM4MFDDGFQ>
.
|
pom.xml
Outdated
</goals> | ||
</execution> | ||
</executions> | ||
</plugin> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this removed? This makes the build not include the plugins. The target/.jar
size isn't 22Mb but 128Kb. And using the REST api to upload the .jar throws <title>Error 400 (Bad Request)!!1</title>
. But even after adding back this part the <title>Error 400 (Bad Request)!!1</title>
still remains.
I build the project with mvn -Dmaven.test.skip=true package
, what can be the problem? Could you help please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, this should not have been removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 thx
Hi, I've built both branches |
I fixed the pom and now it builds correctly. Also, the examples are in a separate PR but will only work on this version. We were asked to separate them out into two PRs. |
This PR contains:
Known Issues:
https://issues.cask.co/browse/PLUGIN-120
Errors in pipeline run on Sandbox environment only.
Note: Examples will be sent in another PR.