On 05/16/2012 04:04 PM, Jozef Zigmund wrote:
On Monday 14 May 2012 16:15:15 ifarkas(a)redhat.com wrote:
> This patchset contains the mustache templates for the 3 remaining jquery
> templates
NACK, as we discussed about found issues by me.
Here is the summary what you would fix:
1. compress lines over 80
2. write specs for added/edited methods of images_controller
The images controller
code is tested in features/image.feature. The show
action is currently uncommented and you can also see the note above
which states that first we need to refactor the usage of VCR to fix it.
There are other efforts to refactor our test suite, and this patchset is
not intended to do that.
3. 'Push all' button in images#show is abled to click when
some providers are
disabled .. for the last point maybe good to ask Angus(if it's ok or not, to
be 100% sure)
The visibility of Push all button is defined by BZ #799981. I think
if
you want the functionality to be changed you should first file a BZ for
that, but probably the button makes more sense as it is. This patchset
is not about changing functionality, but refactor what we currently have.
Otherwise it's great job and everything is working.
I am about to send an updated patchset that fixes the line limit. I also
fixed the catalog checking in deployables_controller you mentioned on
IRC. More discussion on the above are welcome!
Thanks for the review!
Imre