Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scarpe components #355

Merged
merged 5 commits into from
Aug 14, 2023
Merged

Scarpe components #355

merged 5 commits into from
Aug 14, 2023

Conversation

noahgibbs
Copy link
Collaborator

@noahgibbs noahgibbs commented Aug 8, 2023

Description

This adds a scarpe-components gem, with things like the Logging-gem-based logger moved to it. Over time this is where other replaceable code with dependencies will move, since Lacci should be usable with no dependencies at all.

The gem itself will continue having no mandatory dependencies -- display libraries can use components from it, and then declare dependencies on necessary pieces (e.g. the Logging gem.)

Checklist

  • Run tests locally
  • Run linter(check for linter errors)

@noahgibbs noahgibbs force-pushed the scarpe-components branch 2 times, most recently from d432ad8 to 5912212 Compare August 9, 2023 09:53
@noahgibbs noahgibbs force-pushed the scarpe-components branch 2 times, most recently from ee8e530 to d7fe3b5 Compare August 12, 2023 19:41
@noahgibbs noahgibbs marked this pull request as ready for review August 13, 2023 07:49

Lacci is, at heart, an API with as little implementation as possible. It declares the Shoes GUI objects, but not how to display them. Ordinarily a Shoes application will require (in the sense of Kernel#require) every part of Lacci -- it will load the entire library. Even if we add optional components (e.g. a Lacci-based Bloops API with no implementation behind it), those components will be loaded by every Shoes app that uses that part of the API.

As a result, Lacci should have minimal (preferably zero) dependencies. Any code doing "real work" should be removed from Lacci completely, as soon as possible. Since *every* Shoes app is going to require Lacci, it should be possible to use it with essentially no dependencies, minimal memory footprint, minimal load time. It is a skeleton to hang functionality on, not a thing that functions for itself.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fully agree with this. It's one of those things you get the feel of after writing a few features. Like, 'the widget should accept this kwarg but it should default to "20px"', that's firmly in the realm of Lacci.

but what actually happens with that kwarg's value in the display isn't


Scarpe-Components is a grab bag of default implementations, intended to be replaceable. But each of them does something. Most of them have dependencies. It's fine for a Scarpe-Component implementation to depend on the Logging gem, provided it says it does. Nokogiri? 100% fair. Does it do something with a nontrivial amount of computation, like rendering HTML output? No problem. This is very different from Lacci.

If a component should be reused, it's probably fine to put it into Scarpe-Components. It *might* be fine to put it in Lacci. For Scarpe-Components, ask yourself, "will more than one Scarpe display service possibly want to use this?" For Lacci, the test is more strict: "will *every* Scarpe display service want to use this? Does it have no dependencies at all, and do very little computation and take very little memory?"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good point to make. and we will continue to have tough decisions.

we've already found that out with like, I want to write a shoes app but it will only work on MacOS because of code I've chosen to chuck in it.

Or I could see components that 95% are repeatable but have a 5% change between displays and we have to decide whether to split into two or inject if-statements (which I think is fine).

But that's programming, I think this makes sense to support our core values of allowing folks to develop this out into different ways

@noahgibbs noahgibbs merged commit 1e93120 into main Aug 14, 2023
@noahgibbs noahgibbs deleted the scarpe-components branch August 18, 2023 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants