Re: [BuildStream] Proposal: Workspace related DX features & design



Hi,

On Mon, Sep 3, 2018 at 2:59 PM William Salmon <will salmon codethink co uk> wrote:
I think myself and Tom agreed with Sanders last remarks so we have been a bit slow in responding.

As Tristan has been quite on this I amuse he is similarly minded, if not if he could reply sooner rather than later then it will avoid myself and tom codeing this up and then having a too lengthy MR rework.

I have made some notes further down as well.

Responses inline.
 
Thanks All
Will

PS. Tristan i know you are supper busy wit 1.2 atm but if you could fine moment this week, as we will probs start next week that would be really appreciated.


On 28/08/18 23:11, Sander Striker wrote:
Hi,

On Tue, Aug 28, 2018 at 1:12 PM Tom Pollard <tom pollard codethink co uk> wrote:
Hi Sander,

[...] 
>
>     When opening a single element then you can:
>
>     bst workspace open --directory /path/to/workspaces --multi
>     elementA.bst elementB.bst
>
>
> Why do we need the --multi argument?  That doesn't seem intuitive.  If
> it is to get around the fact that otherwise elementB.bst would be
> interpreted as the workspacename, then I don't think we should do that. 
> Otherwise you end up with an open workspace carrying the name of the
> second element you wanted a workspace open for.

Yes it is to mitigate the situation you've mentioned. Will has some
thoughts about handling this with click in alternative ways, however as
it stands the most sane thing seemed to be the use of an additional flag.

I'm opposed to the --multi flag.  I don't think it's intuitive.

I am definitely concerned about this invocation in the current proposal resulting in:
$ bst workspace open elementA.bst elementB.bst
... opens up a workspace for elementA.bst in a directory called /path/to/workspaces/elementB.bst...

I think it makes more sense to in this case make a backward incompatible change to the CLI, for the benefit of a better user experience.

I read this as: we all dislike the --multi flag but we can think of a better way?
So I think we are agreed to go with the --multi/-m flag.

It's beyond dislike.  I really don't think we should do that based on the example I've given above.

[...]
>     The directory option specifies the directory, in which the folder 
>     containing/(the root of) the workspace will be created. The folder 
>     containing/(the root of) the workspace is either the "workspacename"
>     from the 
>     command line or if not given then it is the element name without the
>     trailing
>     ".bst".
>
>     eg. `bst workspace open --directory /path/to/workspaces elementA.bst`
>
>     would create:
>     path/to/workspaces/elementA/
>     as the root or the workspace
>
>     and `bst workspace open --directory /path/to/workspaces element.bst
>     workspacename`
>
>     would create
>     path/to/workspaces/workspacename/

[...] 
> What is the sensible default location used when no workspace location is
> defined?  'workspaces' as a subdirectory of the project directory?  Or
> will not having it set result in a helpful failure message?

My preference would for be the command to behave as is to not break any
compatibility with any existing automated workspaces. As such I'd
propose to keep the existing helpful failure message, all be it reworded
to include the availability of defaults. Having workspaces as
sub-directories of a project is also a workflow in which Tristan has
been an advocate of not recommending (see issue #191), so making this
default behavior would be somewhat challenging that.

I'm ok with challenging Tristan on this one as I am still not convinced that it's that bad (Hi Tristan ;)).  However I am not necessarily opposed to have the helpful failure message with clear instructions how to configure.  What would result in the best user experience?

I think I agree with Tristan on preference but I agree with sander that it doesn't make scene to disable the feature if the user wants it. It would be nice if this was something you could force but not default to tho.. however i think we are all agreed to have the default location if a default has not been set to be the location the command was run. eg, the project root or one of its sub folders.

I'm fine with the default of not being configured, and requiring a one-time action from the user - on the premise that the error message gives clear instructions on how to configure.  I'm assuming this is not too controversial?
[...] 
> What is the recovery mode for a moved workspace that can't find its
> project?  Can it be re-registered?

Will mentioned that a user could 'fudge' the path given in a workspace
fragment which would allow for manual intervention in the case of
re-registering. Ultimately that's a bit messy so going forward with a
utility (a new flag for bst workspace?) for the re-association of
workspaces to projects is probably the right thing to do.

+1.  I don't really like the idea that we end up with an FAQ that tells you to edit the 'fragment'.
 
How 'smart'
this command would be, especially when trying to generate a list of
potential parent projects, is something that would need to be more
fleshed out if desired.

I think we'll be ok if we ask for confirmation to replace all associations with just the association to this single project.

I feel like this is a lot of effort when we could just say "we don't support moving projects", just clone/copy the project with out the .bst folder and set up new workspaces your self.

That said if we want a mover utility function/tidier its not a lot of effort so we could easily right it.

I prefer stating that we don't support moving over suggesting someone "...could 'fudge' the path given in a workspace fragment...".

I imagine from the user perspective this could look something like:
$ cd $project_dir
$ bst workspace close --force $element.bst
$ bst workspace open --directory /new/path/to/element/workspace $element.bst

which I think is acceptable.  The individual workspace commands should already be supported, unless I'm missing something?

[...]
> I feel that half of the proposal is non-contentious and the other half
> is around Debug.  From the above I am not clear there are going to be
> any conflicting workspace CLI changes needed, it seems not.  Can we
> separate out Debug on that basis?

I agree here that the majority of cli changes needed (or as we currently
see as needed) here are for bst shell. However there is definitely
overlaps & the point of using cached build trees (& especially if they
are to become by default what is currently checked out into a workspace)
are places that we feel benefit from being in the overarching design
proposal. If deemed necessary I'd take no issue with the core design &
implementation of extending debugging with bst shell to be separated out.

That would be my preference if there are no objections.

Again I think we all agree but we all needed to have a think about it, and we have, so lets move on.

Good to see we have consensus.

Cheers,

Sander 
--

Cheers,

Sander


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