← Back to team overview

opencompute-developers team mailing list archive

Re: [Merge] lp:~sophia-wu/opencompute/add-ocp-system-event-log-job into lp:opencompute/checkbox

 

Review: Needs Fixing

Hi Sophia.... thanks for submitting this.  In general it looks good but there are a couple things:

1: Why is there a separate whitelist for this?  Does this need it's own whitelist, or would this be in a general certification remote whitelist?  If that's the case, the whitelist should simply be called opencompute-certify-remote.whitelist.

2: if you want the config file me.cfg to be installed, you need to add it to the approrpiate files in the debian directory.  here are teh places where the examples directory is referenced:

debian/checkbox-hw-collection.install:usr/share/checkbox/examples/checkbox-hw-collection.ini
debian/checkbox.install:usr/share/checkbox/examples/checkbox.ini
debian/checkbox.install:usr/share/checkbox/examples/network.cfg
debian/checkbox.install:usr/share/checkbox/examples/virtualization.cfg
debian/checkbox.install:usr/share/checkbox/examples/org.freedesktop.policykit.checkbox.policy usr/share/polkit-1/actions/
debian/checkbox-ocp-cli.install:usr/share/checkbox/examples/checkbox-ocp-cli.ini
debian/checkbox-ocp-gtk.install:usr/share/checkbox/examples/checkbox-ocp-gtk.ini
debian/checkbox-ocp-qt.install:usr/share/checkbox/examples/checkbox-ocp-qt.ini
debian/checkbox-ocp-urwid.install:usr/share/checkbox/examples/checkbox-ocp-urwid.ini
debian/checkbox.postinst:cp /usr/share/checkbox/examples/network.cfg /etc/checkbox.d/
debian/checkbox.postinst:cp /usr/share/checkbox/examples/virtualization.cfg /etc/checkbox.d/

You're probably interested in checkbox.install and checkbox.postinst

Without adding it to those files, the config file will not be installed with the package.

3: in the ipmi_sel_entries script, you reference ../examples/me.cfg twice.  The proper installed location is /etc/checkbox.d/ (See 2 above).

4: Finally, you also need to update debian/changelog and add an entry under your name that briefly describes the changes you've made.  See other entries in the changelog for examples.

Fix those few items (and just let me know more about item 1) and I'll push this through.

Thanks!

-- 
https://code.launchpad.net/~sophia-wu/opencompute/add-ocp-system-event-log-job/+merge/205966
Your team Open Compute Developers is subscribed to branch lp:opencompute/checkbox.


References