|
Description
Lawrence R. Steeger
2008-04-08 19:39:13 UTC
Created attachment 15781 [details] [review] Enable private peer-to-peer message bus access/fix man page Fix some text in dbus-send.c in prior patch. No logic changes. The second version is not much better.
Please
* do not use tabs.
* remove trailing whitespace.
* add a new line before '{'
...
Just compare your code to the rest of dbus-send.c, it's obvious.
Created attachment 15782 [details] [review] Add peer-to-peer message bus access/fix man page/fix coding style Fixed coding style. Sorry, Lawrence, but it seems that the new patch does not match the requirements either. You shall not run indent on the patched file. Just add the bits you want to get reviewed and stick coding style of the existing code. Your latest patch changes the whole file which doesn't help us on reviewing your changes. Please understand that we have to stick to those rules to keep the project's source code in a consistent shape. Thanks you. Created attachment 15801 [details] [review] Add peer-to-peer message bus access/fix man page/fix coding style Hopefully, last coding style request. Please note that the dbus-send.c source from git://anongit.freedesktop.org/git/dbus/dbus contains several coding style errors of its own (i.e., tabs and indentation). This one looks much better, thank you. Havoc, J5: Could you please have a look at the last revision of the patch? Thanks! Thank you for bearing with us. D-Bus is on it's way to becoming an actual standard (it is defacto right now) so we are a little stricter with the code than most other projects. Technically the patch looks good but there are a couple of things that need to be fixed before we can commit: There are a couple of patches in there. You don't have to open another bug but can you split out the expanded error messages that do not relate to the peerbus with the core of your patch? I want to commit them separately with separate ChangeLog entries. Also please attach detailed ChangeLogs (look at the ChangeLog file for format). Peerbus and private aren't descriptive enough. You can open any address even the session bus and system bus with connection_open, as you only have one connection per invocation of dbus_send, public and private connections have no real meaning. A better moniker is simply --address and address as the variable (or connection_address). It might be there but I didn't see it. Can you print out an error if --address is specified and either --session or --system is also specified? And the last point. This patch is significant enough for you to get your name in the AUTHORS file. We also need you to make a statement in this bug that you agree to license this patch under the MIT Permissive license. Though we ship GPL/Artistic all new code must be explicitly licensed under the MIT Permissive license as we are trying to move to that license to eliminate confusion which crops up with our current licensing scheme. Thanks. Created attachment 15813 [details] [review] Add --address=ADDRESS option The --address=ADDRESS option allows direct connection to a specified address. It is mutually exclusive with --session and --system. Patch contents licensed under the MIT Permissive license. Created attachment 15814 [details]
ChangeLog entry for --address=ADDRESS patch
Created attachment 15815 [details]
Add expanded error messages for options and arguments
See ChangeLog attachement for details.
Patch contents licensed under the MIT Permissive license.
Created attachment 15816 [details]
ChangeLog entry for expanded error messages
Patch contents licensed under the MIT Permissive license.
Created attachment 15818 [details] [review] Man page updates for --address=ADDRESS, etal. See ChangeLog entry for details. Patch contents licensed under the MIT Permissive license. Created attachment 15819 [details]
ChangLog entry for man page updates
Patch contents licensed under the MIT Permissive license.
Created attachment 15820 [details] [review] Add expanded error messages for options and arguments See ChangeLog entry for details. Patch contents licensed under the MIT Permissive license. Created attachment 15822 [details] [review] Add --address=ADDRESS option Add --address=ADDRESS option The --address=ADDRESS option allows direct connection to a specified address. It is mutually exclusive with --session and --system. Patch contents licensed under the MIT Permissive license. (small problem in prior patch. lrs) Created attachment 15823 [details] [review] Add --address=ADDRESS option Add --address=ADDRESS option The --address=ADDRESS option allows direct connection to a specified address. It is mutually exclusive with --session and --system. Patch contents licensed under the MIT Permissive license. (small problem in prior patch. lrs) Ok, all done and ready for comments. Thanks. Applied - sorry about the delay. commit 14afa0564e9eea01d28d4b2fd1e6ac0bfec626e7 Author: Lawrence R. Steeger <lsteeger@gmail.com> Date: Sat Oct 18 14:50:49 2008 -0400 Bug 15412: Add --address option to dbus-send Signed-off-by: Colin Walters <walters@verbum.org> |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.