Re: [BuildStream] Merging 'git_tag' and 'git'



Hey Will,


‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, 16 January 2020 20:13, William Salmon via buildstream-list <buildstream-list gnome org> wrote:

Ben

I take issue with some of the points bellow but that's not really the
trust of what I was trying to say, so I will focus on that.


I am sorry if you feel like that, I did not mean to come out aggressive here.

I was trying to say, dose the odd additive feature, really pose such a
big threat to UI consistency?


For me yes. Let me elaborate why:

1) When reading an element, it is important to be able to understand
quickly what is happening in it.

One quick way of understanding that is the element name. Currently,
if I see 'git' element, I know _exactly_ how it will track, what will
happen and what I can expect. I do not need to go and read the whole
configuration of the plugin.

With such changes as proposed here, we would end up having to read more
for some plugins in order to understand exactly what is happening,
which puts more mental burden on the person reading it.

Granted, if you only work with a few elements, that is not too much.
However, depending on the project, you might be exposed to way more elements.
This also makes reviewing changes much harder, since now you need the whole
context of the plugin to understand what changed and whether the change should
be allowed.

2) Documentation. Whenever we add features that are 'optional' on an
element, we add a new branching point in the documentation. This might be
added in multiple places, like "So if the branch is that, we do this, unless you
specify this, in case we do that, unless...". That makes documentation hard to
read and understand, and adds more burden on choosing the plugin you want.

I strongly believe that having more, shorter plugins, with less optional stuff,
make things easier to understand and decide what options/plugins to pick.

We are advocating for people to write smaller BuildStream elements that
does one thing, and does it well. I believe we should do the same for plugins.

3) A point that came up with Tom's cleanup of the _gitsourcebase:

When trying to make an element make too many different things, you often
end up with the choice of:

- Adding a lot of conditionals everywhere, making the code harder to maintain
- Impacting performance for all the different use cases. (One regression, that
got caught during review, would have seen a 100x slow down on the computation
of the 'ref' for the plugin in some cases).

Which means that, expanding the plugin, fixing bugs, or making it more robust,
performant or reliable becomes more tedious.

UI doesn't stop at the CLI. When writing a BST file, you will need to

know

a bit around the plugins that you are using.

As far as I can tell the only effect these would have on users who are
not interested in them is that there would be a few more lines in the
doc labeled optional that they may or may not read. I cant think of how
the new behaviors would effect how the plugin works other than the
cadence of one of its features, or how that would cause confusion as to
`you will need to know a bit around the plugins that you are using`.

That still makes the docs harder to read, write and understand, plus makes
reviewing changes/picking options harder (cf. above)


[...] Having plugins with vastly
different behaviors depending on their configuration is unintuitive[...]

I would completely agree with this, however I am intrigued to hear that
tracking with a different cadence falls in to this category.

I am interested in what aspect(s) of these addition features would have
a negative impact on users of bst that were not interested in them?


I hope my points more up helps shed more lights as to why I think merging
those two plugins is a good idea? Let me know if some poitns are still
unclear and I'll try to make it more explicit.



I would also have one other question that I am still not clear with:

If both plugins were living in the same repository and sharing the
bulk of the logic, would you see any other incentives in merging them?


Thanks
Will


Cheers,
Ben


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