diff mbox

[1/2] btrfs-progs: introduce command namespace for development features

Message ID 79debec8a345c66dd0b02793de680e0438428ee0.1375791411.git.dsterba@suse.cz (mailing list archive)
State Under Review, archived
Headers show

Commit Message

David Sterba Aug. 6, 2013, 12:25 p.m. UTC
We'd like to make it easier to preview a new feature and remove the
burden to invent sane user interface (command name, placement,
arguments, man) from the beginning. For this purpose the developer are
free to use the 1st level namespace called '_'. It will be hidden from
regular btrfs help output and the only way to get the commands is

  btrfs _ --help

The commands appear as if they are in the 1st level, but have to be used
from inside _, eg.

  btrfs _ newcommand

Once the interface is stable, the command will be moved to the right
place.

Signed-off-by: David Sterba <dsterba@suse.cz>
---
 btrfs.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Zach Brown Aug. 6, 2013, 6:01 p.m. UTC | #1
On Tue, Aug 06, 2013 at 02:25:20PM +0200, David Sterba wrote:
> We'd like to make it easier to preview a new feature and remove the
> burden to invent sane user interface (command name, placement,
> arguments, man) from the beginning.

My biggest worry about this is that it complicates the coordination of
automated testing, which is already in a terrible state for btrfs-progs.
It can't possibly motivate people to write tests if we make the process
more cumbersome than it already is.

So we develop tests for a command (maybe in xfstests,  maybe in
btrfs-progs) that use this magical _ namespace.  Then the command is
merged.  When are the tests updated?  Do they fallback to both so that
the tests can work across the merge?   Do we add some complexity to try
and magically match _ commands that aren't found with matching commands
somewhere else in the heirarchy?  Ugh, all 'round.

I'm not sure I understand what problem this is really solving.  People
shouldn't be expecting to find incomplete features in the master branch,
right?  If people are looking to test incomplete work they can get your
integration branch and, well, we don't care if it changes later?

- z
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anand Jain Aug. 7, 2013, 3:10 a.m. UTC | #2
Further there is benefit of having newer subcommands
  in the master branch - it gets visibility.

  However if we are trying to solve the problem of it
  not being end user ready then there can be a warning
  message about it when the user uses it or sees it.

  I see some of online shops tagging new problem with
  bright red "New" on the product image. Could we have
  some "warning" shown against these new subcommands ?

  just my 1c.

Anand


On 08/07/2013 02:01 AM, Zach Brown wrote:
> On Tue, Aug 06, 2013 at 02:25:20PM +0200, David Sterba wrote:
>> We'd like to make it easier to preview a new feature and remove the
>> burden to invent sane user interface (command name, placement,
>> arguments, man) from the beginning.
>
> My biggest worry about this is that it complicates the coordination of
> automated testing, which is already in a terrible state for btrfs-progs.
> It can't possibly motivate people to write tests if we make the process
> more cumbersome than it already is.
>
> So we develop tests for a command (maybe in xfstests,  maybe in
> btrfs-progs) that use this magical _ namespace.  Then the command is
> merged.  When are the tests updated?  Do they fallback to both so that
> the tests can work across the merge?   Do we add some complexity to try
> and magically match _ commands that aren't found with matching commands
> somewhere else in the heirarchy?  Ugh, all 'round.
>
> I'm not sure I understand what problem this is really solving.  People
> shouldn't be expecting to find incomplete features in the master branch,
> right?  If people are looking to test incomplete work they can get your
> integration branch and, well, we don't care if it changes later?
>
> - z
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Aug. 7, 2013, 11:26 a.m. UTC | #3
On Tue, Aug 06, 2013 at 11:01:37AM -0700, Zach Brown wrote:
> My biggest worry about this is that it complicates the coordination of
> automated testing, which is already in a terrible state for btrfs-progs.
> It can't possibly motivate people to write tests if we make the process
> more cumbersome than it already is.
> 
> So we develop tests for a command (maybe in xfstests,  maybe in
> btrfs-progs) that use this magical _ namespace.  Then the command is
> merged.  When are the tests updated?  Do they fallback to both so that
> the tests can work across the merge?   Do we add some complexity to try
> and magically match _ commands that aren't found with matching commands
> somewhere else in the heirarchy?  Ugh, all 'round.

My motivation is to merge various patchsets into one git tree even if
it's not in a final state of development, to let people test use it and
give feedback about usability. Patches floating around in the
mailinglist get tested by very few people.

A test developed for a _ command is unlikely to be merged into xfstests,
I expect the test to land there after the command is fininished
and has fixed command line UI.

In the meantime, the test reflects the _ status of the command. I was
thinking about extending xfstests to allow external testscripts to be
run as if it were a regular xfstest (using all the infrastructure). That
way the test is part of the btrfs-progs patchset and is synchronized
with the command name. Once it's done, _ is dropped and test can be
submitted to xfstests.

> I'm not sure I understand what problem this is really solving.  People
> shouldn't be expecting to find incomplete features in the master branch,
> right?  If people are looking to test incomplete work they can get your
> integration branch and, well, we don't care if it changes later?

Well, I hope no incomplete feature ends up in master, but for example
the chunk-recover got merged. Moving the command name is simple, but we
want to catch it earlier than when it's too late.

I have to re-organize integration branch(es) better, so there is eg. a
branch without unstable stuff, possibly always in a pullable state. On
top of that a bunch of topic branches with the _ features.

Let me know if I missed to answer something important.
david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Aug. 7, 2013, 12:38 p.m. UTC | #4
On Wed, Aug 07, 2013 at 01:26:58PM +0200, David Sterba wrote:
> I have to re-organize integration branch(es) better, so there is eg. a
> branch without unstable stuff, possibly always in a pullable state. On
> top of that a bunch of topic branches with the _ features.

... but then there's no point hiding the commands under _ hmm.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zach Brown Aug. 7, 2013, 6:12 p.m. UTC | #5
On Wed, Aug 07, 2013 at 02:38:12PM +0200, David Sterba wrote:
> On Wed, Aug 07, 2013 at 01:26:58PM +0200, David Sterba wrote:
> > I have to re-organize integration branch(es) better, so there is eg. a
> > branch without unstable stuff, possibly always in a pullable state. On
> > top of that a bunch of topic branches with the _ features.
> 
> ... but then there's no point hiding the commands under _ hmm.

You might not be able to tell from way over there, but I'm making that
gesture where you point at your nose to indicate that you understood my
point exactly :)

- z
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/btrfs.c b/btrfs.c
index 4e93e13..dff0d70 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -239,6 +239,28 @@  static int handle_options(int *argc, char ***argv)
 	return (*argv) - orig_argv;
 }
 
+static const char * const devel_features_cmd_usage[] = {
+	"btrfs _ <command> [<args>]",
+	NULL
+};
+
+const struct cmd_group devel_features_cmd_group = {
+	devel_features_cmd_usage,
+	"WARNING: this is a namespace for commands that are in development\n"
+	"and anything is subject to change. Once the user interface gets\n"
+	"stabilized, it'll be moved to the appropriate place.\n"
+	"NOTE: you have to call the commands as eg.\n"
+	"\tbtrfs _ newcommand --args\n",
+	{
+		{ 0, 0, 0, 0, 0 }
+	}
+};
+
+int cmd_devel_features(int argc, char **argv)
+{
+	return handle_command_group(&devel_features_cmd_group, argc, argv);
+}
+
 const struct cmd_group btrfs_cmd_group = {
 	btrfs_cmd_group_usage, btrfs_cmd_group_info, {
 		{ "subvolume", cmd_subvolume, NULL, &subvolume_cmd_group, 0 },
@@ -255,6 +277,8 @@  const struct cmd_group btrfs_cmd_group = {
 		{ "quota", cmd_quota, NULL, &quota_cmd_group, 0 },
 		{ "qgroup", cmd_qgroup, NULL, &qgroup_cmd_group, 0 },
 		{ "replace", cmd_replace, NULL, &replace_cmd_group, 0 },
+		{ "_", cmd_devel_features, devel_features_cmd_usage,
+			&devel_features_cmd_group, 1 },
 		{ "help", cmd_help, cmd_help_usage, NULL, 0 },
 		{ "version", cmd_version, cmd_version_usage, NULL, 0 },
 		{ 0, 0, 0, 0, 0 }