Skip to content

docs(edge-apps): restructure READMEs with consistent format and tables#686

Merged
nicomiguelino merged 21 commits intomasterfrom
docs/restructure-edge-app-readmes
Feb 16, 2026
Merged

docs(edge-apps): restructure READMEs with consistent format and tables#686
nicomiguelino merged 21 commits intomasterfrom
docs/restructure-edge-app-readmes

Conversation

@nicomiguelino
Copy link
Contributor

@nicomiguelino nicomiguelino commented Feb 12, 2026

User description

Summary

  • Standardize section ordering across all Edge App READMEs (Getting Started, Deployment, Configuration, Development, Testing)
  • Convert settings from bullet lists to markdown tables with Setting, Description, Type, and Default columns
  • Emphasize deployment commands with bun run deploy in deployment steps
  • Simplify and unify Development and Testing sections
  • Remove excessive detail while preserving essential information

Affected Apps

  • edge-apps/simple-timer
  • edge-apps/qr-code
  • edge-apps/menu-board
  • edge-apps/cap-alerting
  • edge-apps/clock
  • edge-apps/weather
  • edge-apps/grafana

PR Type

Documentation


Description

  • Standardize Edge App README structure

  • Add Getting Started and Deployment steps

  • Convert settings lists into tables

  • Clarify CAP playlist priority integration


Diagram Walkthrough

flowchart LR
  n1["README standard structure"] 
  n2["Getting Started / Deployment"] 
  n3["Configuration tables"] 
  n4["CAP playlist priority docs"] 
  n5["Updated app READMEs (7)"]
  n1 -- "applied across" --> n5
  n1 -- "adds" --> n2
  n1 -- "adds" --> n3
  n5 -- "includes" --> n4
Loading

File Walkthrough

Relevant files
Documentation
7 files
README.md
Restructure README and update playlist docs                           
+40/-16 
README.md
Convert configuration docs to settings table                         
+12/-14 
README.md
Standardize setup steps and config table                                 
+11/-21 
README.md
Rewrite README and document default items                               
+45/-105
README.md
Add standard sections and config table                                     
+14/-18 
README.md
Replace settings list with markdown table                               
+6/-4     
README.md
Update deploy commands and configuration table                     
+13/-15 

- Standardize section ordering across all Edge App READMEs
- Convert settings from bullet lists to tables with Setting, Description, Type, and Default columns
- Emphasize deployment commands with `bun run deploy` in deployment steps
- Simplify and unify Development and Testing sections
- Remove excessive detail while preserving essential information
@github-actions
Copy link

github-actions bot commented Feb 12, 2026

PR Reviewer Guide 🔍

(Review updated until commit 089a99e)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Doc Accuracy

The configuration table changes documented defaults/expectations (e.g., theme default now shown as light, and enable_analytics default shown as true). Please validate these match the actual app defaults and configuration schema so users don’t end up with unexpected behavior.

| Setting                  | Description                                                                                                                                                   | Type               | Default        |
| ------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | -------------- |
| `enable_analytics`       | Enable or disable Sentry and Google Analytics integrations                                                                                                    | optional, global   | `true`         |
| `openweathermap_api_key` | OpenWeatherMap API key to access weather data and location information. Get your API key from <https://openweathermap.org/api>                                | optional, global   | -              |
| `override_locale`        | Override the default locale with a supported language code                                                                                                    | optional           | `en`           |
| `override_timezone`      | Override the default timezone with a supported timezone identifier (e.g., `Europe/London`, `America/New_York`). Defaults to the system timezone if left blank | optional           | -              |
| `tag_manager_id`         | Google Tag Manager container ID to enable tracking and analytics                                                                                              | optional, global   | `GTM-P98SPZ9Z` |
| `theme`                  | Visual theme for the app: 'light' or 'dark'                                                                                                                   | required           | `light`        |
| `unit`                   | Measurement unit for temperature display: `auto` (automatically determined based on location), `metric` (Celsius), or `imperial` (Fahrenheit)                 | optional, advanced | `auto`         |
API Semantics

The new “Playlist Priority Integration” guidance references PATCH /v4/playlists and suggests toggling priority: true/false. Please confirm the endpoint path and payload are correct for the Playlist API (e.g., whether an ID is required in the path and whether priority is the right field and type) so the integration instructions are actionable.

## Playlist Priority Integration

This app is designed to work with Screenly's Playlist API to automatically interrupt regular content when emergency alerts are active. Configure your backend to call the [`PATCH /v4/playlists`](https://developer.screenly.io/api_v4/#update-a-playlist) endpoint with `priority: true` when new CAP alerts are detected, which will cause this app to take precedence over other content. Set `priority: false` when alerts expire to resume normal playlist rotation.
Clarity

The configuration tables use a “Type” column with values like “required”, “optional, global”, and “optional, advanced”, which reads more like “Requirement/Scope” than a data type. Consider renaming the column(s) or adjusting the values to avoid confusion (e.g., separate columns for Required, Scope, and Data type).

| Setting                  | Description                                                                                                                                                   | Type               | Default |
| ------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | ------- |
| `enable_analytics`       | Enable or disable Sentry and Google Analytics integrations                                                                                                    | optional, global   | -       |
| `openweathermap_api_key` | OpenWeatherMap API key to access weather data and location information. Get your API key from <https://openweathermap.org/api>                                | required, global   | -       |
| `override_coordinates`   | Comma-separated coordinates (e.g., `37.8267,-122.4233`) to override device location                                                                           | optional           | -       |
| `override_locale`        | Override the default locale with a supported language code                                                                                                    | optional           | `en`    |
| `override_timezone`      | Override the default timezone with a supported timezone identifier (e.g., `Europe/London`, `America/New_York`). Defaults to the system timezone if left blank | optional           | -       |
| `sentry_dsn`             | Sentry DSN for error tracking and monitoring                                                                                                                  | optional, global   | -       |
| `tag_manager_id`         | Google Tag Manager container ID to enable tracking and analytics                                                                                              | optional, global   | -       |
| `unit`                   | Measurement unit for temperature display: `auto` (automatically determined based on location), `metric` (Celsius), or `imperial` (Fahrenheit)                 | optional, advanced | `auto`  |

@github-actions
Copy link

github-actions bot commented Feb 12, 2026

PR Code Suggestions ✨

Latest suggestions up to 089a99e
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Make analytics opt-in by default

Documenting analytics as enabled by default and including a specific GTM container
ID can cause unintentional data collection and misconfiguration. Change the defaults
to “unset/disabled” so users explicitly opt in by providing their own values.

edge-apps/clock/README.md [25-29]

-| `enable_analytics`       | Enable or disable Sentry and Google Analytics integrations                                                                                                    | optional, global   | `true`         |
-| `tag_manager_id`         | Google Tag Manager container ID to enable tracking and analytics                                                                                              | optional, global   | `GTM-P98SPZ9Z` |
+| `enable_analytics`       | Enable or disable Sentry and Google Analytics integrations                                                                                                    | optional, global   | `false`        |
+| `tag_manager_id`         | Google Tag Manager container ID to enable tracking and analytics                                                                                              | optional, global   | -              |
Suggestion importance[1-10]: 5

__

Why: Making enable_analytics opt-in and removing a hardcoded tag_manager_id is a reasonable privacy/security improvement in documentation, reducing accidental tracking. However, it may diverge from the app’s actual defaults unless the underlying implementation also changes.

Low
Possible issue
Fix required/default contradiction

The table marks menu_title as required while also providing a default, which is
contradictory and can mislead users configuring screenly.yml. Mark it as optional
(since a default exists) or remove the default if it truly must be provided.

edge-apps/menu-board/README.md [30]

-| `menu_title`       | The title displayed at the top of the menu | required           | `Today's Menu`              |
+| `menu_title`       | The title displayed at the top of the menu | optional           | `Today's Menu`              |
Suggestion importance[1-10]: 4

__

Why: The menu_title row is marked required while also listing a default, which is potentially confusing for users configuring screenly.yml. This is a minor documentation consistency improvement, but it depends on whether the app truly requires an explicit value.

Low

Previous suggestions

Suggestions up to commit 635d946
CategorySuggestion                                                                                                                                    Impact
General
Restore key integration documentation

Re-add the Override Playlist guidance (or at least a short section + link), since
it’s a core operational dependency for automatically interrupting regular content.
Without it, users can deploy the app but still miss the key integration needed for
expected behavior.

edge-apps/cap-alerting/README.md [38-42]

 ### Nearest Exit Tags
 
 Add tags to your Screenly screens (e.g., `exit:North Lobby`) to provide location-aware exit directions. The app substitutes `{{closest_exit}}` or `[[closest_exit]]` placeholders in alert instructions.
 
+## Override Playlist Integration
+
+This app is designed to work with Screenly's [Override Playlist API](https://developer.screenly.io/api-reference/v4/#tag/Playlists/operation/override_playlist) to interrupt regular content when alerts are active. Configure your backend to call the API when new CAP alerts are detected.
+
 ## Development
Suggestion importance[1-10]: 6

__

Why: The PR removed the Override Playlist Integration section from edge-apps/cap-alerting/README.md, and restoring at least a short link can prevent users from missing a key integration for real-world operation. This is a meaningful documentation completeness improvement, though not a code-level correctness fix.

Low
Possible issue
Add explicit app directory step

Add an explicit step to run commands from the app directory; otherwise users running
from the repo root may install the wrong dependencies (or fail entirely in a
monorepo). This is especially important now that the README no longer includes a cd
edge-apps/cap-alerting step.

edge-apps/cap-alerting/README.md [5-7]

 ```bash
+cd edge-apps/cap-alerting
 bun install
<details><summary>Suggestion importance[1-10]: 5</summary>

__

Why: The updated README no longer mentions running commands from `edge-apps/cap-alerting`, which can indeed cause confusion or failures in a monorepo. Adding an explicit `cd` step improves usability/documentation correctness, but it’s not a functional/code issue.


</details></details></td><td align=center>Low

</td></tr></tr></tbody></table>

</details>

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request standardizes the documentation structure across seven Edge App READMEs, improving consistency and readability. The changes restructure each README to follow a uniform format with Getting Started, Deployment, Configuration, Development, and Testing sections, convert settings from bullet lists to markdown tables, and emphasize the use of bun run deploy for deployment.

Changes:

  • Standardized section ordering across all affected Edge Apps
  • Converted settings from bullet lists to structured markdown tables with Setting, Description, Type, and Default columns
  • Updated deployment commands to consistently use bun run deploy instead of screenly edge-app deploy
  • Simplified Development and Testing sections to focus on essential commands
  • Removed detailed feature descriptions, design notes, and best practices to streamline documentation

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
edge-apps/weather/README.md Restructured sections, converted settings to table format, updated deployment commands
edge-apps/simple-timer/README.md Converted settings to table format with consistent Type and Default columns
edge-apps/qr-code/README.md Complete restructure with standardized sections, removed Features section, converted settings to table
edge-apps/menu-board/README.md Major simplification removing extensive documentation, standardized structure with settings table
edge-apps/grafana/README.md Restructured sections, removed Features and example configuration, converted settings to table
edge-apps/clock/README.md Converted settings to table format, updated deployment commands, simplified Development section
edge-apps/cap-alerting/README.md Significant simplification removing NWS formatting details and override playlist integration info, standardized structure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

nicomiguelino and others added 9 commits February 12, 2026 14:11
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Fix theme setting: change type from optional to required
- Fix theme default value from 'dark' to 'light'
- Add missing default values: enable_analytics ('true'), override_locale ('en'), tag_manager_id ('GTM-P98SPZ9Z')

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Restore NWS Text Product Formatting section with period-based forecasts and WWWI format documentation
- Fix setting names to match manifest: default_language → language, maximum_alerts → max_alerts

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add "Default Menu Items" section documenting the 4 sample pizzas
- Add missing currency and display_errors settings from manifest
- Fix menu_title type from optional to required to match manifest

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add description explaining integration with Screenly's Playlist API
- Add dedicated "Playlist Priority Integration" section with details on using PATCH /v4/playlists endpoint
- Document how to use priority parameter to interrupt content during alerts

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Apply prettier formatting to improve markdown table alignment

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@nicomiguelino nicomiguelino marked this pull request as ready for review February 12, 2026 23:03
@github-actions
Copy link

Persistent review updated to latest commit 089a99e

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

nicomiguelino and others added 5 commits February 12, 2026 15:08
Break long line into shorter sentences with bullet points to comply with MD013 linting rule (400 char limit)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Fix headline and url from optional to required
- Add default values for all settings (call_to_action, enable_utm, headline, url)
- Add missing display_errors setting
- Mark enable_utm as advanced

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add display_errors setting from manifest to configuration table

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

salmanfarisvp and others added 2 commits February 13, 2026 06:54
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@salmanfarisvp
Copy link
Collaborator

@nicomiguelino Also added new app in Repo Readme and fix the app order - 7f86395

nicomiguelino and others added 2 commits February 13, 2026 07:40
- Remove enable_analytics, tag_manager_id, and sentry_dsn from configuration tables
- Keep openweathermap_api_key as it's needed for local development and CLI deployment
- Remove "global" designation from openweathermap_api_key type

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add item_X_description, item_X_labels, item_X_name, and item_X_price to configuration table
- Remove redundant "Menu Items" subsection
- Format table descriptions to wrap at 100 characters

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

nicomiguelino and others added 2 commits February 13, 2026 08:40
- Change item_X_* to item_XX_* to reflect actual zero-padded implementation
- Clarify that XX is 01-25, not 1-25
- Add examples to show correct usage (e.g., item_01_description)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@nicomiguelino nicomiguelino merged commit 4c75b78 into master Feb 16, 2026
11 checks passed
@nicomiguelino nicomiguelino deleted the docs/restructure-edge-app-readmes branch February 16, 2026 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants