dart-lang / build

A build system for Dart written in Dart
https://pub.dev/packages/build
BSD 3-Clause "New" or "Revised" License
791 stars 211 forks source link

docs/examples: build_to missing from cssBuilder #3674

Closed ewann closed 6 months ago

ewann commented 7 months ago

Off the back of https://www.youtube.com/watch?v=ngUiuIdcGjk I was reviewing the examples. After longer than I would have liked, I noticed this omission.

Happy to raise a pr, but didn't easily find contrib rules so decided to file here in the first instance - hopefully if nothing else happens it will at least be on a next persons search results.

diff --git a/example/build.yaml b/example/build.yaml
index 6d18b367..0b9831fe 100644
--- a/example/build.yaml
+++ b/example/build.yaml
@@ -24,6 +24,7 @@ builders:
     import: "package:example/builder.dart"
     builder_factories: ["cssBuilder"]
     build_extensions: {"$package$": ["web/generated.css"]}
+    build_to: source
     auto_apply: root_package

   textBuilder:
jakemac53 commented 7 months ago

This isn't a required field - the result of leaving it empty just means that it doesn't build to "source" it builds to "cache" (under .dart_tool/build/generated/...).

The main difference is whether it is checked into the repo by default or not (or for a package, whether it is published with the package or not). For this example, it probably doesn't particularly matter which option is chosen. We should maybe have an explicit choice though, with a comment about what it means?

ewann commented 6 months ago

perhaps my real question is: what is this example supposed to demonstrate?

All of the other builder examples in https://github.com/dart-lang/build/blob/master/example/build.yaml build_to: source but https://github.com/dart-lang/build/blob/master/example/build.yaml#L23 does not.

So I think I'd be agreeing by stating that having // build_to: source would be helpful to a new reader, since it hopefully calls out louder: A setting is available here, but is deliberately not configured. That would possibly have brought the difference to my attention faster, thus improving the learning / onboarding process.

jakemac53 commented 6 months ago

I agree I think the fix here is just to call out the choice with a comment.