allow templates for new apps/blades/libraries to be pluggable to improve scaffolding options #126

Closed
andyberry88 opened this Issue Oct 9, 2013 · 15 comments

Comments

Projects
None yet
5 participants
@andyberry88
Member

andyberry88 commented Oct 9, 2013

Create a mechanism (similar to http://yeoman.io/) so devs can write their own templates and users can select which template to use for a new app/blade etc

For the first pass we don't need to create anything too clever. Pointing at a template directory or URL should be fine. The logic capabilities of the template definitions should not be the subject of discussion for this first pass.

Maybe a full template is just a template app with all supported Asset Containers defined and each command (create-app, create-bladeset, create-blade, create-library) just uses the appropriate one. I believe we already have a mechanism similar to this anyway.

@leggetter

This comment has been minimized.

Show comment
Hide comment
@leggetter

leggetter Oct 10, 2013

Contributor

Templates are also discussed here #3

Yeoman generators are used for scaffolding applications. You might - for instances - create a BRJS Yeoman generator that would understand BRJS templates, but also BRJS application structure.

In our case we just want templates as the application structure is defined by the BRJS model.

Contributor

leggetter commented Oct 10, 2013

Templates are also discussed here #3

Yeoman generators are used for scaffolding applications. You might - for instances - create a BRJS Yeoman generator that would understand BRJS templates, but also BRJS application structure.

In our case we just want templates as the application structure is defined by the BRJS model.

@leggetter leggetter changed the title from allow templates for new apps/blades etc to be pluggable to allow templates for new apps/blades/libraries etc to be pluggable Sep 29, 2014

@leggetter leggetter added this to the 0.14 milestone Sep 29, 2014

@leggetter leggetter changed the title from allow templates for new apps/blades/libraries etc to be pluggable to allow templates for new apps/blades/libraries to be pluggable to improve scaffolding options Sep 29, 2014

@andyberry88 andyberry88 added the size 3 label Oct 13, 2014

@ioanalianabalas ioanalianabalas added 2 - Dev and removed 1 - Planned labels Nov 18, 2014

@ioanalianabalas ioanalianabalas self-assigned this Nov 18, 2014

@andyberry88

This comment has been minimized.

Show comment
Hide comment
@andyberry88

andyberry88 Nov 18, 2014

Member

After a design discussion with @dchambers @james-shaw-turner and @ioanalianabalas we've decided on the following:

  • create-app, create-bladeset, create-blade & create-library will all accept an optionaly --template=<template> arg that specifies the template to use
  • if the --template arg isnt provided the default will be used
  • all templates will live in conf/templates and templates will be named <template-name>-<asset-container-type> - e.g. angular-blade, angular-bladeset etc.
  • the template name for the 'default' template will be ommited

@leggetter has suggested some changes to this plan (comments welcome):

  • templates should be in a sub directory with the template name. e.g. /conf/templates/angular/blade, /conf/templates/angular/bladeset etc.
  • Using templates from a GitHub repo URL/path should also be possible - e.g. --template=<repo-url>
  • as a 'would like to have' it would be nice to have an entire app structure as a template rather than having to break it down into it's constituent parts (this is probably not likely to happen in the near future)

Unless anyone has any objections it would seem sensible to include the first 2 of @leggetter's suggestions in the initial plan for template support.

Member

andyberry88 commented Nov 18, 2014

After a design discussion with @dchambers @james-shaw-turner and @ioanalianabalas we've decided on the following:

  • create-app, create-bladeset, create-blade & create-library will all accept an optionaly --template=<template> arg that specifies the template to use
  • if the --template arg isnt provided the default will be used
  • all templates will live in conf/templates and templates will be named <template-name>-<asset-container-type> - e.g. angular-blade, angular-bladeset etc.
  • the template name for the 'default' template will be ommited

@leggetter has suggested some changes to this plan (comments welcome):

  • templates should be in a sub directory with the template name. e.g. /conf/templates/angular/blade, /conf/templates/angular/bladeset etc.
  • Using templates from a GitHub repo URL/path should also be possible - e.g. --template=<repo-url>
  • as a 'would like to have' it would be nice to have an entire app structure as a template rather than having to break it down into it's constituent parts (this is probably not likely to happen in the near future)

Unless anyone has any objections it would seem sensible to include the first 2 of @leggetter's suggestions in the initial plan for template support.

@dchambers

This comment has been minimized.

Show comment
Hide comment
@dchambers

dchambers Nov 18, 2014

Contributor

@andyberry88 / @leggetter, I think:

  1. is clearly a good idea, and should be done from the get-go.
  2. is maybe a good idea, but shouldn't be something we do in the initial release.
  3. is maybe a good idea, but also not for the initial release.
Contributor

dchambers commented Nov 18, 2014

@andyberry88 / @leggetter, I think:

  1. is clearly a good idea, and should be done from the get-go.
  2. is maybe a good idea, but shouldn't be something we do in the initial release.
  3. is maybe a good idea, but also not for the initial release.
@andyberry88

This comment has been minimized.

Show comment
Hide comment
@andyberry88

andyberry88 Nov 18, 2014

Member

is maybe a good idea, but shouldn't be something we do in the initial release.

Adding this would mean the getting started guide could be much more straightforward since the first command would be brjs create-app --template=github.com/repos/BladeRunnerJS/getting-started rather than the user having to clone a repo or download a template into the right location themselves.
Since theres a Java Git client which makes this reasonably straightforward to achieve IMO we should do it for 1.0

I agree 3 should be pushed back until post 1.0.

Member

andyberry88 commented Nov 18, 2014

is maybe a good idea, but shouldn't be something we do in the initial release.

Adding this would mean the getting started guide could be much more straightforward since the first command would be brjs create-app --template=github.com/repos/BladeRunnerJS/getting-started rather than the user having to clone a repo or download a template into the right location themselves.
Since theres a Java Git client which makes this reasonably straightforward to achieve IMO we should do it for 1.0

I agree 3 should be pushed back until post 1.0.

@dchambers

This comment has been minimized.

Show comment
Hide comment
@dchambers

dchambers Nov 19, 2014

Contributor

Why not include a getting-started template in the BRJS release, and then it becomes even simpler:

brjs create-app --template=getting-started
Contributor

dchambers commented Nov 19, 2014

Why not include a getting-started template in the BRJS release, and then it becomes even simpler:

brjs create-app --template=getting-started
@ioanalianabalas

This comment has been minimized.

Show comment
Hide comment
@ioanalianabalas

ioanalianabalas Dec 1, 2014

Contributor

@dchambers, @james-shaw-turner and I had a further discussion on how we handle implicitly called user-templates (e.g. default aspect and default bladeset are created implicitly by the 'populate' method in app) if the user wants to use a template that has only been defined for the up-most level (e.g. app in this case). We have talked about 3 ways in which to handle this issue:

  1. Create empty folders
  2. Populate the child entities based on the "default" template.
  3. Throw a Runtime exception if any of the child entity templates do not exist.

We have decided on option 3 for now, and we'll see in the future if there is a requirement to change this.

Contributor

ioanalianabalas commented Dec 1, 2014

@dchambers, @james-shaw-turner and I had a further discussion on how we handle implicitly called user-templates (e.g. default aspect and default bladeset are created implicitly by the 'populate' method in app) if the user wants to use a template that has only been defined for the up-most level (e.g. app in this case). We have talked about 3 ways in which to handle this issue:

  1. Create empty folders
  2. Populate the child entities based on the "default" template.
  3. Throw a Runtime exception if any of the child entity templates do not exist.

We have decided on option 3 for now, and we'll see in the future if there is a requirement to change this.

@ioanalianabalas

This comment has been minimized.

Show comment
Hide comment
@ioanalianabalas

ioanalianabalas Dec 2, 2014

Contributor

Adding new acceptance criteria as per the 02/12/2014 stand-up with @andyberry88, @dchambers, @leggetter, @thecapdan:

  1. If the requested template exists in both conf/templates and sdk/conf/templates, the template from conf/templates will be used.
  2. If the requested template exists in both conf/templates and sdk/conf/templates and a template is missing from conf/templates, the template of the same name from sdk/conf/templates will be used.
  3. j2eeify will be moved from its previous location in sdk/templates to the sdk root.
Contributor

ioanalianabalas commented Dec 2, 2014

Adding new acceptance criteria as per the 02/12/2014 stand-up with @andyberry88, @dchambers, @leggetter, @thecapdan:

  1. If the requested template exists in both conf/templates and sdk/conf/templates, the template from conf/templates will be used.
  2. If the requested template exists in both conf/templates and sdk/conf/templates and a template is missing from conf/templates, the template of the same name from sdk/conf/templates will be used.
  3. j2eeify will be moved from its previous location in sdk/templates to the sdk root.
@ioanalianabalas

This comment has been minimized.

Show comment
Hide comment
@ioanalianabalas

ioanalianabalas Dec 5, 2014

Contributor

Please see #1109.

Contributor

ioanalianabalas commented Dec 5, 2014

Please see #1109.

@thecapdan

This comment has been minimized.

Show comment
Hide comment
@thecapdan

thecapdan Dec 18, 2014

Contributor

Started testing this. In general it seems fine but I have found an issue:

When I created a template to override the default aliases file in a blades resources folder. When I create a new bladevia the command line with the template flag:

danielo@SPARE-T440P /d/repo/brjs/brjs-sdk/workspace/sdk (user-templates-126)
$ brjs create-blade brjstomoo tomo boss --template=yikes
Successfully created new blade 'boss'
  D:\repo\brjs\brjs-sdk\workspace\apps\brjstomoo\tomo-bladeset\blades\boss

It does indeed use the template i defined for the aliases file. However it fails to populate any of the other folders. (boss blade using new aliases template, hogg blade uses default)

image

I would expect that if it cannot find a template specified for the template attribute, then it should revert to the default templates.

Contributor

thecapdan commented Dec 18, 2014

Started testing this. In general it seems fine but I have found an issue:

When I created a template to override the default aliases file in a blades resources folder. When I create a new bladevia the command line with the template flag:

danielo@SPARE-T440P /d/repo/brjs/brjs-sdk/workspace/sdk (user-templates-126)
$ brjs create-blade brjstomoo tomo boss --template=yikes
Successfully created new blade 'boss'
  D:\repo\brjs\brjs-sdk\workspace\apps\brjstomoo\tomo-bladeset\blades\boss

It does indeed use the template i defined for the aliases file. However it fails to populate any of the other folders. (boss blade using new aliases template, hogg blade uses default)

image

I would expect that if it cannot find a template specified for the template attribute, then it should revert to the default templates.

@thecapdan thecapdan added 2 - Dev and removed 5 - Test labels Dec 18, 2014

@dchambers

This comment has been minimized.

Show comment
Hide comment
@dchambers

dchambers Dec 18, 2014

Contributor

@thecapdan, this is by design. I think @ioanalianabalas was maintaining a set of acceptance criteria somewhere, but can't seem to locate them.

Contributor

dchambers commented Dec 18, 2014

@thecapdan, this is by design. I think @ioanalianabalas was maintaining a set of acceptance criteria somewhere, but can't seem to locate them.

@thecapdan

This comment has been minimized.

Show comment
Hide comment
@thecapdan

thecapdan Dec 18, 2014

Contributor

Ok, I accept that my issue described above was not how this was intended to work. Which is fine.... IF -there is a way for me to satisfy my usecase:)

I would expect - if i create a 'default' folder in the templates folder, then any templates that I create in there should override the default template from the sdk, but revert to the sdk versions if they have not been defined in conf.

@ioanalianabalas can you point me at the acceptance criteria that was defined during previous discussions.

Contributor

thecapdan commented Dec 18, 2014

Ok, I accept that my issue described above was not how this was intended to work. Which is fine.... IF -there is a way for me to satisfy my usecase:)

I would expect - if i create a 'default' folder in the templates folder, then any templates that I create in there should override the default template from the sdk, but revert to the sdk versions if they have not been defined in conf.

@ioanalianabalas can you point me at the acceptance criteria that was defined during previous discussions.

@ioanalianabalas

This comment has been minimized.

Show comment
Hide comment
@ioanalianabalas

ioanalianabalas Dec 18, 2014

Contributor

This is what was agreed upon during the design meeting on 11/12/2014:

  1. If the requested template exists in both conf/templates and sdk/conf/templates, the template from conf/templates will be used.
  2. If the requested template exists in both conf/templates and sdk/conf/templates and a template is missing from conf/templates, the template of the same name from sdk/conf/templates will be used.
  3. j2eeify will be moved from its previous location in sdk/templates to the sdk root.
  4. If present, the default templates in conf/templates/default will override the default templates in sdk/conf/templates/default.
  5. If no template is specified, the default template will be used (see 3.).
  6. A TemplateNotFoundException will be thrown if the template for the main entity being created is not found (e.g. the app template for create-app, the bladeset for create-bladeset).
  7. If the template for an implicitly created entity does not exist, an empty folder will be created (e.g. test-unit for bladeset).
Contributor

ioanalianabalas commented Dec 18, 2014

This is what was agreed upon during the design meeting on 11/12/2014:

  1. If the requested template exists in both conf/templates and sdk/conf/templates, the template from conf/templates will be used.
  2. If the requested template exists in both conf/templates and sdk/conf/templates and a template is missing from conf/templates, the template of the same name from sdk/conf/templates will be used.
  3. j2eeify will be moved from its previous location in sdk/templates to the sdk root.
  4. If present, the default templates in conf/templates/default will override the default templates in sdk/conf/templates/default.
  5. If no template is specified, the default template will be used (see 3.).
  6. A TemplateNotFoundException will be thrown if the template for the main entity being created is not found (e.g. the app template for create-app, the bladeset for create-bladeset).
  7. If the template for an implicitly created entity does not exist, an empty folder will be created (e.g. test-unit for bladeset).
@thecapdan

This comment has been minimized.

Show comment
Hide comment
@thecapdan

thecapdan Dec 18, 2014

Contributor

Thanks @ioanalianabalas

So point 2 satisfies my usecase and i've tested to confirm it works. Can confirm points 1-7 works. Tested all using a user defined template for the aliases.xml in the blade.

Contributor

thecapdan commented Dec 18, 2014

Thanks @ioanalianabalas

So point 2 satisfies my usecase and i've tested to confirm it works. Can confirm points 1-7 works. Tested all using a user defined template for the aliases.xml in the blade.

@thecapdan

This comment has been minimized.

Show comment
Hide comment
@thecapdan

thecapdan Dec 18, 2014

Contributor

Will merge once the conflicts are sorted

Contributor

thecapdan commented Dec 18, 2014

Will merge once the conflicts are sorted

@thecapdan thecapdan added 6 - Done and removed 5 - Test labels Dec 29, 2014

@thecapdan

This comment has been minimized.

Show comment
Hide comment
@thecapdan

thecapdan Dec 29, 2014

Contributor

merged

Contributor

thecapdan commented Dec 29, 2014

merged

@andyberry88 andyberry88 modified the milestones: 1.0 M1, 0.15 Jan 28, 2015

@andyberry88 andyberry88 removed the 6 - Done label Jan 28, 2015

@andyberry88 andyberry88 added the feature label Feb 17, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment