Update site #41
Update site #41
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds API functionality to make the live server work as a "slave" to build tools, allowing external build tools to trigger refresh events and control the development server lifecycle.
- Adds dezombify functionality to kill processes on specified ports
- Introduces API to accept external refresh topics for build tool integration
- Refactors code organization by extracting configuration and CLI options into separate files
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| sjsls/src/dezombify.scala | New utility to kill processes on ports with OS-specific implementations |
| sjsls/src/refreshRoute.scala | Updates refresh handling to support external build tools |
| sjsls/src/LiveServerConfig.scala | Extracts config case class with dezombify option |
| sjsls/src/CliOpts.scala | Extracts CLI option definitions |
| plugin/src/refresh_plugin.scala | New mill plugin for ScalaJS refresh integration |
| sjsls/src/routes.scala | Updates to pass build tool parameter |
| sjsls/test/src/dezombify.test.scala | New test for dezombify functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
sjsls/src/refreshRoute.scala
Outdated
| ) | ||
| case _ => | ||
| case GET -> Root / "refresh" / "v1" / "sse" => | ||
| println("Hit this one") |
There was a problem hiding this comment.
Debug println should be removed or replaced with proper logging using the logger parameter.
| println("Hit this one") | |
| logger.debug("Hit this one") |
Copilot uses AI. Check for mistakes.
sjsls/src/htmlGen.scala
Outdated
| def vanillaTemplate(styles: Boolean): String = | ||
| val r = Ref.of[IO, Map[String, String]](Map.empty) | ||
| r.flatMap( rf => | ||
| vanillaTemplate(styles, rf, false).map(_.render) | ||
| ).unsafeRunSync()(using cats.effect.unsafe.implicits.global) |
There was a problem hiding this comment.
Using unsafeRunSync in a synchronous API can block threads and cause performance issues. Consider making this function return IO[String] instead.
| def vanillaTemplate(styles: Boolean): String = | |
| val r = Ref.of[IO, Map[String, String]](Map.empty) | |
| r.flatMap( rf => | |
| vanillaTemplate(styles, rf, false).map(_.render) | |
| ).unsafeRunSync()(using cats.effect.unsafe.implicits.global) | |
| def vanillaTemplate(styles: Boolean): IO[String] = | |
| val r = Ref.of[IO, Map[String, String]](Map.empty) | |
| r.flatMap( rf => | |
| vanillaTemplate(styles, rf, false).map(_.render) | |
| ) |
Copilot uses AI. Check for mistakes.
| "Whether or not to attempt killing any processes that are using the specified port. Default: true" | ||
| ) | ||
| .orTrue |
There was a problem hiding this comment.
The dezombify flag defaults to true with .orTrue, which means the flag is enabled by default. This is counterintuitive - flags typically default to false and are enabled when specified.
| "Whether or not to attempt killing any processes that are using the specified port. Default: true" | |
| ) | |
| .orTrue | |
| "Whether or not to attempt killing any processes that are using the specified port." | |
| ) | |
| .orFalse |
Copilot uses AI. Check for mistakes.
| case Some(foundEt) => | ||
| if etag == foundEt then | ||
| logger.debug(s"ETag $etag found in cache at path ${req.uri.path}, returning 304") >> | ||
| logger.debug("map is: " + map.toString) >> |
There was a problem hiding this comment.
String concatenation with + operator for logging can be expensive if the map is large. Consider using string interpolation or structured logging.
| logger.debug("map is: " + map.toString) >> | |
| logger.debug(s"map is: $map") >> |
Copilot uses AI. Check for mistakes.
1 participant
Mill 1.
Adds an API which will slave this to a build tool