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

-----Original Message----- From: kimchi-devel-bounces(a)ovirt.org [mailto:kimchi-devel-bounces@ovirt.org] On Behalf Of Paulo Ricardo Paz Vital Sent: segunda-feira, 5 de outubro de 2015 14:10 To: sguimaraes943(a)gmail.com; Kimchi Devel <kimchi-devel(a)ovirt.org&gt; 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(a)gmail.com wrote: _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel