Alon Bar-Lev has posted comments on this change. Change subject: userportal, webadmin: remove PatternFly from source tree ......................................................................
Patch Set 1: (7 comments) nice! http://gerrit.ovirt.org/#/c/29018/1/Makefile File Makefile: Line 445 Line 446 Line 447 Line 448 Line 449 please keep one empty line before Line 445: Line 446: install -d -m 755 "$(DESTDIR)$(PKG_SYSCONF_DIR)/branding" Line 447: -rm -f "$(DESTDIR)$(PKG_SYSCONF_DIR)/branding/00-ovirt.brand" Line 448: ln -s "$(DATA_DIR)/branding/ovirt.brand" "$(DESTDIR)$(PKG_SYSCONF_DIR)/branding/00-ovirt.brand" Line 449: -rm -f "$(DATA_DIR)/branding/ovirt.brand/patternfly" $(DESTDIR) missing Line 450: ln -s /usr/share/patternfly/resources "$(DATA_DIR)/branding/ovirt.brand/patternfly" Line 451: ln -sf "$(DATA_DIR)/conf/osinfo-defaults.properties" "$(DESTDIR)$(PKG_SYSCONF_DIR)/osinfo.conf.d/00-defaults.properties" Line 452: Line 453: gwt-debug: Line 446: install -d -m 755 "$(DESTDIR)$(PKG_SYSCONF_DIR)/branding" Line 447: -rm -f "$(DESTDIR)$(PKG_SYSCONF_DIR)/branding/00-ovirt.brand" Line 448: ln -s "$(DATA_DIR)/branding/ovirt.brand" "$(DESTDIR)$(PKG_SYSCONF_DIR)/branding/00-ovirt.brand" Line 449: -rm -f "$(DATA_DIR)/branding/ovirt.brand/patternfly" Line 450: ln -s /usr/share/patternfly/resources "$(DATA_DIR)/branding/ovirt.brand/patternfly" $(DESTDIR) is missing please also add PATTERNFLY_RESOURCES_DIR as variable at top of make, to allow customization, for example people that wants to use this from $HOME Line 451: ln -sf "$(DATA_DIR)/conf/osinfo-defaults.properties" "$(DESTDIR)$(PKG_SYSCONF_DIR)/osinfo.conf.d/00-defaults.properties" Line 452: Line 453: gwt-debug: Line 454: [ -n "$(DEBUG_MODULE)" ] || ( echo "Please specify DEBUG_MODULE" && false ) http://gerrit.ovirt.org/#/c/29018/1/README.developer File README.developer: Line 23: - jboss-as (optional) Line 24: - maven-3 (optional) Line 25: - pyflakes (optional) Line 26: - python-pep8 / pep8 (optional) Line 27: - patternfly please document this as optional, and document that PATTERNFLY_RESOURCES_DIR may be used to customize. Line 28: Line 29: Maven-3 is required, download and extract if not installed using Line 30: distribution package management, and add to PATH. Line 31: http://gerrit.ovirt.org/#/c/29018/1/ovirt-engine.spec.in File ovirt-engine.spec.in: Line 739: Line 740: rm -f "%{engine_data}/branding/ovirt.brand/patternfly" Line 741: rm -f "%{engine_etc}/branding/00-ovirt.brand" Line 742: ln -s /usr/share/patternfly/resources "%{engine_data}/branding/ovirt.brand/patternfly" Line 743: ln -s /usr/share/patternfly/resources "%{engine_etc}/branding/00-ovirt.brand" why do we need this in Makefile and here? don't you need to add Require statement? how come you need resources at two places? Line 744: Line 745: # Line 746: # Register services Line 747: # http://gerrit.ovirt.org/#/c/29018/1/packaging/branding/ovirt.brand/common.css File packaging/branding/ovirt.brand/common.css: Line 1 Line 2 Line 3 why these cannot be mered into upstream? Line 1: @import url("gwt_common.css"); Line 2: @import url("patternfly/dist/css/patternfly.min.css"); the resources should not include "dist" directory, it should be the "dist" directory. so something wrong in the packaging. Line 3: @import url("patternfly-ovirt.css"); Line 4: @import url("patternfly-custom-hacks.css"); Line 5: Line 6: /* LoginPopupView.ui.xml: -- To view, visit http://gerrit.ovirt.org/29018 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1da6bece78951a98be148050078c6d8201018273 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Greg Sheremeta <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Einav Cohen <[email protected]> Gerrit-Reviewer: Greg Sheremeta <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
