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

Adding archetype for Siddhi extensions #1

Merged
merged 3 commits into from
Nov 6, 2017
Merged

Adding archetype for Siddhi extensions #1

merged 3 commits into from
Nov 6, 2017

Conversation

kalaiyarasiganeshalingam
Copy link
Contributor

No description provided.


```
<dependency>
<groupId>org.wso2.extension.siddhi.io.</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

io should change to store

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 624c01a


```
<dependency>
<groupId>org.wso2.extension.siddhi.io.</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

io should be changed to execution

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 624c01a

package ${package}.${executionTypeinLowerCase}.streamprocessor;

public class TestCaseOf${classNameOfStreamProcessor} {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

format

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 624c01a

* name = "The name of the extension",
* namespace = "The namespace of the extension",
* description = "The description of the extension (optional).",
* //Sink configurations
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not sink. Check all other places where this comment is used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 624c01a

```
<dependency>
<groupId>org.wso2.extension.siddhi.io.</groupId>
<artifactId>siddhi-io-</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

io should be replaced by map

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 624c01a

* name = "The name of the extension",
* namespace = "The namespace of the extension",
* description = "The description of the extension (optional).",
* //Sink configurations
Copy link
Contributor

Choose a reason for hiding this comment

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

sink mapper configs. Check all other places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 624c01a


```
<dependency>
<groupId>org.wso2.extension.siddhi.io.</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

io -> script

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 624c01a

@tishan89 tishan89 merged commit 384579b into siddhi-io:master Nov 6, 2017
</plugins>
</pluginManagement>
</build>
</project>

Choose a reason for hiding this comment

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

Please add a new line at the end of each file. Please correct in all the palces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in #12

<extension>
<groupId>org.apache.maven.archetype</groupId>
<artifactId>archetype-packaging</artifactId>
<version>3.0.1</version>

Choose a reason for hiding this comment

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

Normal practice is to add version as properties to parent pom and refer that here. Can't we use the same here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in #12

@@ -0,0 +1,117 @@
<?xml version="1.0" encoding="UTF-8"?>

Choose a reason for hiding this comment

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

Missing license header

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the code review comment, all files under the archetype-resources folder don't need to put the license header. Because these files are a resource for generating the project using the archetype. Hence I didn't put the license header.

@@ -0,0 +1,109 @@
<?xml version="1.0" encoding="UTF-8"?>

Choose a reason for hiding this comment

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

Missing license file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the code review comment, all files under the archetype-resources folder don't need to put the license header. Because these files are a resource for generating the project using the archetype. Hence I didn't put the license header.

@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>

Choose a reason for hiding this comment

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

license header? Please correct in all the places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the code review comment, all files under the archetype-resources folder don't need to put the license header. Because these files are a resource for generating the project using the archetype. Hence I didn't put the license header.

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.

3 participants