On Fri, 2010-09-24 at 23:21 +0200, [email protected] wrote:
> From: Jan Provaznik <[email protected]>
> 
> ---
>  src/app/controllers/templates_controller.rb        |  200 
> ++++++++------------
>  src/app/util/image_descriptor_xml.rb               |   31 +++
>  src/app/views/templates/_basics.haml               |   31 ++--
>  src/app/views/templates/_build_images.haml         |    5 -
>  src/app/views/templates/_content_selection.haml    |   24 +++
>  src/app/views/templates/_groups.haml               |   11 -
>  src/app/views/templates/_image_formats.haml        |    7 -
>  src/app/views/templates/_managed_content.haml      |   10 +
>  src/app/views/templates/_nav.haml                  |   16 --
>  src/app/views/templates/_packages.haml             |    6 -
>  src/app/views/templates/_platform.haml             |   12 --
>  src/app/views/templates/_scripting.haml            |   10 -
>  src/app/views/templates/_services.haml             |   23 ---
>  src/app/views/templates/_software.haml             |   85 ---------
>  src/app/views/templates/_targets.haml              |   18 --
>  src/app/views/templates/_templates.haml            |   45 +++--
>  src/app/views/templates/build_form.haml            |   27 +++
>  src/app/views/templates/builds.haml                |   44 +++++-
>  src/app/views/templates/index.haml                 |   27 +++-
>  src/app/views/templates/new.haml                   |   50 ++++-
>  src/app/views/templates/packages.haml              |    8 -
>  src/app/views/templates/services.haml              |   38 ----
>  src/app/views/templates/software.haml              |   95 ---------
>  src/app/views/templates/summary.haml               |   61 ------
>  src/app/views/templates/targets.haml               |    1 -
>  .../image_descriptor_platform_repositories.yml     |    4 +
>  src/config/image_descriptor_targets.yml            |    8 +-
>  27 files changed, 330 insertions(+), 567 deletions(-)
>  delete mode 100644 src/app/views/templates/_build_images.haml
>  create mode 100644 src/app/views/templates/_content_selection.haml
>  delete mode 100644 src/app/views/templates/_groups.haml
>  delete mode 100644 src/app/views/templates/_image_formats.haml
>  create mode 100644 src/app/views/templates/_managed_content.haml
>  delete mode 100644 src/app/views/templates/_nav.haml
>  delete mode 100644 src/app/views/templates/_packages.haml
>  delete mode 100644 src/app/views/templates/_platform.haml
>  delete mode 100644 src/app/views/templates/_scripting.haml
>  delete mode 100644 src/app/views/templates/_services.haml
>  delete mode 100644 src/app/views/templates/_software.haml
>  delete mode 100644 src/app/views/templates/_targets.haml
>  create mode 100644 src/app/views/templates/build_form.haml
>  delete mode 100644 src/app/views/templates/packages.haml
>  delete mode 100644 src/app/views/templates/services.haml
>  delete mode 100644 src/app/views/templates/software.haml
>  delete mode 100644 src/app/views/templates/summary.haml
>  delete mode 100644 src/app/views/templates/targets.haml
> 
This one is also mostly good, know you have plans to make some fixes.
One thing that needs fixing is the cucumber test suite.  The following
three tests break as a result of this patch (quick fixes, I am sure):
* cucumber features/template.feature:11 # Scenario: Add basic info to a
new Template
* cucumber features/template.feature:24 # Scenario: Add a package to the
template
* cucumber features/template.feature:32 # Scenario: Remove a package
from the template

Also, this or the previous patch, cause a ton of spec failures (too many
to put here).  Since this is your first pass, I expect you have just not
touched the tests for updating yet.

Couple other issues I noticed:
* when you are creating a new template, and you add a piece of software,
when the page refreshes, you have lost all the other information you
entered into the form.
* One create template page, the 'name' and 'description' fields have no
default text to indicate what is supposed to go there (as the other
fields do)
* This is more of a usability concern for the html-only version that may
need to be brought up with Jakub or someone else from the UX team.  When
you add packages via the 'add' link, since the page refreshes, you can
only add one package at a time.  This seems very cumbersome to me.  I
know when we add the ajax bits, it will just update the page inline, but
I think we will need some sort of fallback that allows a group add of
some kind.

If you can work in these fixes, I'll check for patch updates over the
weekend.  If you don't get to it, I'll review new version first thing on
Monday.

-j

_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel

Reply via email to