Zbigniew Jędrzejewski-Szmek píše v St 24. 09. 2014 v 18:15 +0200: > > I don't understand the name "method_add_install_dependency_unit_files". > Why not just "method_add_dependencies"? After all, this is not like install, > and does not work on the level of unit files, but units. > > Similarly for the dbus method name "AddInstallDependencyUnitFiles". > This works with unit files, similarly as 'enable', so I would really like to keep the "UnitFiles" part there. And the same for "Install" part, I wanted to make it obvious that this is somehow an alternative to [Install] section in unit file.
> > - r = unit_file_load(c, info, path, root_dir, > > allow_symlink); > > + if (load) > > + r = unit_file_load(c, info, path, > > root_dir, allow_symlink); > > + else > > + r = access(path, F_OK) ? -errno : 0; > Wouldn't it be nicer to push the 'load' parameter into > unit_file_load(), and do the check there? I think that with > your patch the support for root_dir is missing. Yep you are right, this will not work with --root, but I don't think that we want to have function unit_file_load(...., bool really_load). Maybe it would be better to add function unit_file_exists. > > + " based on preset > > configuration\n" > Extra line now? ;) for (int i=0; i<100; i++) puts("Will double check my patches before sending them"); > Zbyszek But really thanks for review. Lukas _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel