-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[WIP] Incorporate the official templates in core repository #500
base: develop
Are you sure you want to change the base?
Conversation
I think it causes some confusions when new users try to learn PIO and found that they will need to clone the template from another repository. One question that I will bring up is: |
We should locate templates under |
@@ -0,0 +1,202 @@ | |||
|
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.
Is the license file necessary for each template after they are incorporated into this repository?
templates/template-scala-parallel-recommendation/data/import_eventserver.py
Show resolved
Hide resolved
Please refer to | ||
https://predictionio.apache.org/templates/recommendation/quickstart/. | ||
|
||
## Versions |
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.
We will have to maintain README of each template after incorporating, I think it should be synchronized with PredictionIO release, but text-classifier template has inconsistent versioning with PredictionIO version. How should we handle that?
In addition, text-classifier template contains large data in its repository, I want to separate that data from the repository. Is there any suitable place to put them?
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 think the test you added to check_templates.sh
is a good idea.
People will know that they have to change the corresponding template version while modifying PIO's version.
As for the data, maybe we add another non-test branch?
@@ -43,4 +43,6 @@ sbt/sbt dataJdbc/compile test storage/test \ | |||
-Delasticsearch.version=$PIO_ELASTICSEARCH_VERSION \ | |||
-Dhbase.version=$PIO_HBASE_VERSION | |||
|
|||
./tests/check_templates.sh |
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.
Add incorporated templates here to check by TravisCI.
@@ -0,0 +1,57 @@ | |||
#!/bin/bash |
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.
This script is run after unit tests on TravisCI to check version consistency and test templates.
@takezoe , sorry for the late response. Basically, I agree with incorporating them into the main repository. I'm concerned about the following before this PR.
|
I'm considering about incorporate the official templates with main repository to reduce maintenance costs of them. My idea is:
make-distribution.sh
.Currently, for every release, we have to update versions of PredictionIO/Scala/Spark/etc, then create tag for each template repository by hand. If the official templates are in the same repository with PredictionIO itself, we will be able to finish this boring operation at once. Also, it will ensure that all official templates is synchronized with the latest PredictionIO version.