GNOME.org
 

Re: [BuildStream] Breaking change: changing '--force or fail' to y/n prompts

On Thu, Dec 13, 2018 at 9:04 AM Tristan Van Berkom
<tristan vanberkom codethink co uk> wrote:


Hi Angelos,

I'm going to snip almost everything out here because it is very long
and I cannot easily structure my reply to it...


Heya Tristan,

Thanks for replying to that long email so quickly! I spent quite some
time trying to make it shorter, unfortunately that was the best I came
up with.


Let me start (for other's sakes) by adding here that the whole reason
we got here, is because through discussion on issue #957, we noticed
that there are commands which require the user to specify `--force`,
and there are commands which prompt the user, and there is no real
reason why we are treating these two categories of activities
differently.

The conclusion of that is that we should be consistent across the
board, and behave the same way whenever we are unsure about doing
something that might be possibly destructive.


Again, my position is that these things are different and can be handled
differently. 'bst workspace reset' always asks you 'Are you sure?',
which is harmful to the user in my opinion. I think it shouldn't ask.

If we were to make 'bst workspace reset' consistent with the other
commands then you would always have to say '--force', or it would
exit with an error. That seems like odd behaviour to me :)


[...]

Hopefully you'll prefer to stick with the simpler '--force or fail',
and instead remove the all 'are you sure?' prompts, let me know!


As you know, I strongly disagree with this, I think it is exactly the
opposite of user friendly, because it forces the user to learn things
which they otherwise would not need to know.

I think whenever the action BuildStream is performing might be
destructive, the default should always be to ask the user interactively
and hold their hand, rather then forcing them to learn a new command
line argument which they might have the burden of remembering in the
future. I consider all the knowledge I need to know about a tool that I
use to be a burden, the less I need to know as a user, the better.

As you also pointed out yourself in the relevant issue, the fact that
sometimes we can only determine whether an activity might be
destructive after loading the pipeline, can also be quite frustrating
if it takes a few seconds to load the pipeline.


Sorry for requiring the repetition, and thanks for bringing this to
the wider forum.


For the rest of your mail, here are a few points:

  o You are proposing a *lot* of options, due to our failure to find
    an option name which clearly reflects the opposite of "-f/--force".

    While I think we both agree that "-f/--force" is a desirable choice
    due to it being very commonly used in command line tools, I don't
    think we should resort to adding multiple options which have the
    same meaning.

    Something that I think is rather important to the user experience
    is how they learn the tool using `bst <command> --help` and man
    pages, crowding this output with multiple options which have the
    same meaning would be very undesirable I think.


I think the redundancy actively aids understanding - "oh
'--assume-yes' is just like regular '--force' from other commands".

I think long options are good for making scripts readable, so I'm
sure you're

I think the short options '--yes' and maybe '-y', and maybe '-f' are
kinder to folks that are typing this stuff often. Possibly we'd just
have '-f', I haven't checked if that's available on every command yet.

It seems like three options '--assume-yes', '--force', and '-f' wouldn't
be so much to learn. Is it this you're objecting to?


  o The buildstream.conf changes you suggest present a weird
    inconsistent interface in my opinion.

    First of all, I think it is a pity that the existing keys start
    with a "really-" prefix, the mere fact that these keys are a part
    of a "prompt" group I think is context enough.


I agree in the logical sense.

I think of it as a bit "tongue in cheek". As someone who wants that
config option, I find the prompts frustrating. The 'really-' prefix makes
me feel that my use-case is understood.


    Secondly, the proposed new members have "bst-" prefixes which I
    think is unnecessarily lengthy for a configuration file which is
    already configuring the behavior of the "bst" command.


I agree - I think we can drop the 'bst' prefixes.


    Note that the entire "prompt" group in the user configuration
    is added in the development branch and was never in a release as
    far as I can see, changing these keys would not even be a breaking
    change, so it would be good to have consistency here too.


*nods*


  o I should point out that the activity (4) which you discard, i.e.
    abort everything right away and exit; should still be accessible
    in the interactive prompts.

    This "abort everything right away and exit" is communicated to
    BuildStream when replying ^C to any prompt.


I would say 'defer' rather than 'discard' - note that I mentioned
'--assume-abort' for later, which would come with an explicit
'abort' option. (Which should probably be 'terminate' instead).


I think overall, for my part I would like a more consistent user
configuration, and I would like to agree on a single word to reflect
the opposite of "-f/--force", finding good names is hard, but this
again comes down to cognitive burden (as a user I don't want the tool
to tell me there are 5 ways to do a thing, that is telling me more
information than I want to know about).

Maybe I should suggest some options:

 --no-force

  You are right the meaning of this is ambiguous, it might mean
  "ask me", but on the other hand it is consistent with how other
  options are expressed, e.g.: `--interactive/--no-interactive`,
  `--strict/--no-strict`, `--verbose/--no-verbose`, etc.

  I wouldn't write it off completely just for it's ambiguity, as it has
  some consistency and I think is still much better than the addition
  of so many different new options.


I think you're perhaps suggesting this set of options:

o --force, -f
o --no-force
o --require-confirmation, --ask

Instead of this set:

o --assume-yes, --yes, --force, -f
o --assume-no, --no
o --require-confirmation, --ask

So you'd be eliminating:

o --assume-yes
o --yes
o --no
o --ask

Does that sound about right?

I'd be very happy to eliminate options (preferably only keeping
'--force' and '-f' :)

As I said, I also like the symmetry of '--no-force' with '--force'. I
consider that
secondary to it being ambiguous though, I think that rules it out as useful.


 --safe

  This I think would communicate that "everything BuildStream does
  will be safe", i.e. no possibly destructive actions would be taken.


I think this is my suggestion! :)

I dropped it because it again isn't as clear as '--force' and friends.

I think maybe we're disagreeing over whether symmetry is more
important than clarity of meaning. I'm on 'team meaning'.


I personally don't like `--assume-yes` and `--assume-no` at all, and
the reason for this is because these names are based on the prompt and
the question itself - this feels ambiguous, and the user would need to
have *seen* the question before hand to every know what they want to
reply to it.


I agree with this - I don't like those same things on apt-get or yum either.

I think they're the least-worst option if we must have prompts.


In other words, I think it is backwards to think that the motivation
for adding these options, is to get rid of pesky prompts, rather we
should have names which communicate to BuildStream what to do, and the
fact that the user is asked something when BuildStream doesn't know
what to do, is just an added convenience to the user.


Hmm, I don't understand this paragraph. Please could you say it
again a different way?

Predictably I do disagree with the 'added convenience' part though,
I think it's an added inconvenience.


Cheers,
    -Tristan


Cheers!
Angelos

(This time reply-all, my mistake)
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]