[BuildStream] About breaking the CLI [was: Bst workspace - UX feedback and change suggestions]



Forking this response as it is mostly orthogonal to the constructive
criticisms highlighted by Phillip.

It needs to be made very clear that the CLI API is a stable API
surface, as we have announced in our initial 1.0 release and have been
advertising it to be. There are 4 stable API surfaces in BuildStream,
they are announced in our very first NEWS item and in the release
announcement for 1.0.0:

    https://mail.gnome.org/archives/buildstream-list/2018-January/msg00000.html

The fact that BuildStream does not currently have a statement in the
documentation including the guarantees we provide, and the guarantees
we do not yet provide but intend to eventually (like stability of
artifact cache keys) is also a problem. That is a different subject but
we are looking into rectifying that during the coming cycle, or in the
context of the 1.2 release if we are lucky.

More comments inline...

On Tue, 2018-08-07 at 09:11 +0100, Daniel Silverstone via BuildStream-list wrote:
On Tue, Aug 07, 2018 at 14:47:53 +0900, Tristan Van Berkom via BuildStream-list wrote:
[...]
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.

I'm going to prefix this with: maybe it will be worthwhile to break the
CLI once for the sake of adding a `--porcelain` like option, which
currently would allow us some flexibility in the stdout of commands
like `bst workspace list` and `bst show`, this is more important now
that we're looking into other commands which produce parsable output in
the new `bst artifact` subgroup:

    https://mail.gnome.org/archives/buildstream-list/2018-July/msg00051.html

With such a break, it might make sense to remove the deprecated 
`--track-save` option to `bst build` and start over with a clean slate.

With that said; we should break it once and only once, with the
intention of never breaking it again.

The CLI is definitely designed specifically with scriptability in mind,
and keeping this interface stable is paramount to avoiding scope creep,
by ensuring that it is safe to use and extend BuildStream's
functionality in various contexts.

My reply to the build manifest proposal last week is also somewhat
relevant, as I am encouraging scriptability of BuildStream in place of
implementing features which different users might have strong,
differing preferences about, but can all have their own way as long as
we provide a nice simple set of tools which allows them to script it:

    https://mail.gnome.org/archives/buildstream-list/2018-August/msg00013.html

One might think that the workspaces are special because they are for
developers and not normally used in production pipelines; but we have
already seen a very nice demo of BuildStream integrated into an IDE,
and that happened around last November or December. If we are to break
API once, I consider the onus to be on us to reach out to those who we
know are using the API and assist them in transition.

It is also important for the GNOME Builder IDE, who are currently
working on integrating BuildStream, that we provide API stability in
our CLI, regarding workspaces and any other CLI interfaces.

I refer back to the prefix of this block, maybe it really does make
sense to break the CLI once in the earlier stages of the project.

If we're going to break the CLI, we better think it over very hard and
be sure that we're satisfied with the change: we must change things
once with the intention of never changing them again.

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.

Again, re above: Workspaces are already automated in some places and
work is ongoing to do more of this. We have guaranteed this stability,
so if we break it once, we had better announce it very strongly.
Regardless of the embarrassment it may cause us, it might be the right
thing.

Also note that afaics, a `--porcelain` like option approach only buys
us freedom in how we report things in stdout. Currently we only output
things which scripts might parse through stdout; all UI-only stuff is
reported through stderr.

I don't think it makes sense to maintain a completely separate set of
CLI commands, for the purpose of only keeping one set API stable.

It *might* make sense to offer an alternative GUI frontend which is for
humans only (the frontend abstraction is almost clean enough for that
to be straight forward, actually).


With all of that said, if there is not a lot of push back against a
single API break for the CLI from other list members, I would support a
well thought out redesign of the workspaces CLI behaviors and the
addition of a `--porcelain` like option (maybe we can give it a better
name though ? --machine-readable ?) to buy us some freedom in stdout,
if someone is going to look into that I suggest the following reading
materials to inform the design:

    Allow configuring default location for workspaces
    https://gitlab.com/BuildStream/buildstream/issues/229

    Allow running bst commands from project sub-directories
    (implemented, but the discussion can be helpful)
    https://gitlab.com/BuildStream/buildstream/issues/368

    Allow running bst commands from workspace directory
    https://gitlab.com/BuildStream/buildstream/issues/222

    Allow project relative workspace paths
    https://gitlab.com/BuildStream/buildstream/issues/191

    Allow workspace commands to work on multiple elements
    https://gitlab.com/BuildStream/buildstream/issues/362


There may be more related requests and discussion I've missed, but any
work in this direction should be guided by a single master plan and API
design in mind, and keeping in mind that what is implemented in the
1.3.x cycle will be frozen in 1.4.

And I would be *very* pleased if this did end up happening.

Cheers,
    -Tristan



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]