[Kimchi-devel] ["PATCH"] Adding python-libsass and new-ui SCSS source to WOK

Mon Oct 5 18:53:10 UTC 2015

-----Original Message-----
From: kimchi-devel-bounces at ovirt.org [mailto:kimchi-devel-bounces at ovirt.org] On Behalf Of Paulo Ricardo Paz Vital
Sent: segunda-feira, 5 de outubro de 2015 14:10
To: sguimaraes943 at gmail.com; Kimchi Devel <kimchi-devel at ovirt.org>
Subject: Re: [Kimchi-devel] ["PATCH"] Adding python-libsass and new-ui SCSS source to WOK

Hello Samuel.

I found some issues regarding your patch here:

0. (optional) Would be nice if you split your patch in multiple commits that work in different parts of the code. For example, all the changes in spec files, Makes, etc can be one commit separeted from the "real"
source code. With that, we can read easier your code and give a better feedback. 

	Doing right now. 

BTW, I could not apply your patch :-( Spliting the patch you prevent this. Here is error of the patch cmd output:

checking file ui/libs/bootstrap-select/bootstrap-select.min.js
patch: **** malformed patch at line 18035:   j in
i)i.hasOwnProperty(j)&&(h.options[j]=i[j])}else{var
k=a.extend({},f.DEFAULTS,a.fn.selectpicker.defaults||{},e.data(),i);e.d
ata("selectpicker",h=new f(this,k,c))}"string"==typeof b&&(g=h[b]instanceof Function?h[b].apply(h,d):h.options[b])}});return"undefined"!=typeof
g?g:h}a.expr[":"].icontains=function(c,d,e){return
b(a(c).text(),e[3])},a.expr[":"].aicontains=function(c,d,e){return
b(a(c).data("normalizedText")||a(c).text(),e[3])};var
f=function(b,c,d){d&&(d.stopPropagation(),d.preventDefault()),this.$ele
ment=a(b),this.$newElement=null,this.$button=null,this.$menu=null,this.
$lis=null,this.options=c,null===this.options.title&&(this.options.title
=this.$element.attr("title")),this.val=f.prototype.val,this.render=f.pr
ototype.render,this.refresh=f.prototype.refresh,this.setStyle=f.prototy
pe.setStyle,this.selectAll=f.prototype.selectAll,this.deselectAll=f.pro
totype.deselectAll,this.destroy=f.prototype.remove,this.remove=f.protot
ype.remove,this.show=f.prototype.show,this.hide 

	I think this is because this line is too big. I'm going to separate this change in one patch to prevent this from happening again. 

1. I saw that you included in all new files the sentence "Code derived from Project Kimchi" in the header. It's not necessary for new Wok files. We use this sentence only for those files we migrated from old Kimchi to Wok framework (not the Kimchi plugin).

	Line-charts and Hosts CSS files were derived from Kimchi, so I guess it is ok to keep them. I'll remove from the other files.

2. I don't see changes on Debian files to add the new package you are working (libsass), and could not found the package libsass (or any other reference to this name) in Fedora and OpenSUSE repositories.
Please provide more information about how to install.

	Libsass is installed by pip (https://pypi.python.org/pypi/libsass). I don't know if there's any other way to include Python packages as dependencies if they don't aren't listed in Fedora or any other distro repository. I found the following workaround:
	-Install python-pip;
	-During Wok make/install issue a "pip install libsass" command;
	-Instead of using python-pip, we can change to NodeJS. If we move to NodeJS Libsass, we can also use Bower to manage UI assets and we don't have to keep bootstrap-sass and bootstrap-select-sass original source in src/vendor folder.

3. Not sure, but I guess I saw some MIT license disclosure in the middle of the code. All copyright and license disclosures must be in top of the file (header).

	Some of these comments with MIT license were from Bootstrap. In the source files (.scss) they are located on top, but when I import them using @import function, they are wrapped in the middle of the CSS files. I'm going to change the the comments from '/* */' to '//', this way sass compiler will ignore them in the final CSS output.

Best Regards,
Paulo

On Sat, 2015-10-03 at 15:19 -0300, sguimaraes943 at gmail.com wrote:
> From: samhenri <samuel.guimaraes at eldorado.org.br>
> 
> *Added python-libsass port as build dependency (MIT license) *Moved 
> bootstrap-select.min.css to libs/bootstrap-select/dist/css folder and 
> updated Makefiles *Added SASS / SCSS source files needed to build the 
> new-ui CSS (Bootstrap and Bootstrap-Select customizations + 
> non-library related / Wok and Kimchi specific styles) *Updated ui/css 
> Makefile to compile SCSS sources into CSS *Updated configure.ac to 
> support changes above *Updated .gitignore to ignore compiled CSS files
> 
> Signed-off-by: samhenri <samuel.guimaraes at eldorado.org.br>
> ---
>  .gitignore                                         |   3 +
>  configure.ac                                       |   3 +
>  contrib/wok.spec.fedora.in                         |   1 +
>  contrib/wok.spec.suse.in                           |   1 +
>  .../plugins/kimchi/contrib/kimchi.spec.fedora.in   |   1 +
>  src/wok/plugins/kimchi/contrib/kimchi.spec.suse.in |   1 +
>  ui/css/Makefile.am                                 |   6 +-
>  ui/css/src/bootstrap-select.custom.scss            |  67 ++
>  ui/css/src/bootstrap.custom.scss                   |  82 ++

_______________________________________________
Kimchi-devel mailing list
Kimchi-devel at ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel