GNOME.org
 

Re: [BuildStream] Bst workspace - UX feedback and change suggestions

On Tue, Aug 07, 2018 at 14:47:53 +0900, Tristan Van Berkom via BuildStream-list wrote:

The reason I say this is because a workspace does not seem to be useful
for a "local" source. When someone tries using a workspace following
our examples, the fact that it's a local source can probably cause a
lot of confusion to the user about what workspaces are for.


I think for an ongoing approach your comments make good sense here.  I think
that, for a first pass, what was done was okay :-)


From this I got the following feedback and reactions:

1) `bst workspace open` was not clear that it would automatically
create the named directory if it did not exist.
    This resulted in making the process feel more clunky and is the
opposite of `bst init` which does not create a named directory, which
makes the UX inconsistent.


I don't think the answer here is to strive for symmetry with `bst init`
(although bst init most certainly creates the project directory, when
one is specified, like `bst -C <project_dir> init ...`), I think that
`bst init` is sufficiently special.


I will say that, as someone new to `bst` a couple of weeks ago, the lack of
consistency between `bst init foo` (errors) and `git init foo` (works, makes
foo) and `bzr init foo` (works, makes foo) tripped me up more than once.  My
fingers (as a long-time 3-character tool user) expect `XXX init foo` to "just
work" :-)  (Though this is not the topic for Phillip's testing, so I'll shush
for now).


I.e., if we're going to populate the workspace directory with the
content of what would be staged for that element; I think it makes
sense to expect we will create the directory.


Perhaps if we're told the name of a dir which exists and is empty, we could
reuse it?


It would however be unfortunate change the CLI behavior in this regard,
because this is not merely UX, workspaces are scriptable (I've heard it
used this way, not sure how much that it's used in an automated
context).


I think anyone who uses a CLI app whose commands are so clearly aimed at
humans, which lacks any `--porcelain` style switches, would accept that
changing from one release to another may require them to rework their
automation slightly.


2)  The closing of a workspace was not intuitive.
    The user expected the directory to be deleted when closing the
workspace.
    And for the keeping of the directory to be the flag, not deletion
of it.

    A potential fix to this could be:
    Diff the closing workspace, with a temporary new workspace, to
see if any changes were made.
    If no changes were made, remove the directory.
    If changes are detected, display a prompt asking if the user
wishes to continue deleting all changes made, and to re-run the
command using `--keep-dir` if they wished to keep the directory.


Sort of ditto to the above point, it would be unfortunate to break the
CLI at this point.


Again ditto re: lack of `--porcelain`.


I would be more happy about changing these two default behaviors if:

  o Others ring in and strongly agree that these proposed defaults
    are better.


My feeling is that the changes proposed would potentially improve matters,
though I have other nits around the workspace UI which I feel more strongly
about (`bst workspace close somedir` should, IMO, work, if `somedir` is a
known workspace, for example).


  o People who do use workspace features in an automated context, agree
    that adjusting their automation scripts to deal with a breaking
    change is not too disruptive to them.


Again, my hope would be that noone would be using workspace features who
hadn't already accepted to themselves that the UI for these commands is
primarily meant for humans and so with a lack of --porcelain would be
remiss to expect certain behaviours across releases.


3) `bst workspace list` displays `[]` when no workspaces are found,
instead of showing a message like "No workspaces found".
    This would be far clearer than simply printing an empty list.



We should definitely add a user facing message to the stderr for this
purpose, removing the `[]` which goes to stdout is not that easily
done.

The stdout of `bst workspace list`, being YAML, is fairly human
readable when workspaces do exist; but it is a stable API surface which
also targets an automation setting - scripts will be parsing the output
of `bst workspace list`, and I think an empty list is more friendly
than nothing at all.

Rather, if we are going to break anything here, again I'd like to hear
from anyone who is using the automation first; and if we do make a
change, I would propose that we expose an option that informs bst that
we are talking to a machine and not a human (in a similar way to how
git makes this distinction with "--porcelain").


See above for my opinions on this :-)


I believe there are more UX features that could be addressed beyond
that of workspaces, and would encourage others to do a similar
process with other new and existing features,
as I feel it helped a lot to get a fresh perspective and remove my
own bias as a factor.



Indeed, and thanks again for the report !


I second this, thank you Phillip.

D.

-- 
Daniel Silverstone                          https://www.codethink.co.uk/
Solutions Architect               GPG 4096/R Key Id: 3CCE BABE 206C 3B69
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]