PMM-15028 Flatten dashboards/, move pmm-app into the ui/ monorepo - #5720
Draft
fabio-silva wants to merge 11 commits into
Draft
PMM-15028 Flatten dashboards/, move pmm-app into the ui/ monorepo#5720fabio-silva wants to merge 11 commits into
fabio-silva wants to merge 11 commits into
Conversation
Restructures developer experience around Grafana dashboards and the QAN Grafana plugin (pmm-app): - Flatten dashboards/dashboards/* up to dashboards/, so the top-level dashboards/ folder holds only JSON dashboard definitions (plus misc/ tooling and packaging metadata required by the RPM spec). - Move pmm-app from dashboards/pmm-app into ui/apps/pmm-app as a member of the existing ui/ Yarn workspaces + Turborepo monorepo, alongside pmm and pmm-compat. The plugin id/dist layout are unchanged. - Recreate the src/dashboards symlink at its new location, now pointing at the flattened top-level dashboards/ folder — plugin.json's include paths keep working unchanged. - Fix pmm-app's lint/test scripts so they're non-mutating/one-shot (required once folded into a shared workspace pipeline), matching the pmm-compat app's conventions. - Add a chokidar-based watcher (scripts/watch-dashboards.mjs) that mirrors dashboard JSON edits into the provisioned dashboards directory during local dev, so both dashboard JSON and QAN JS/TS changes show up without any manual reload command. Wired into pmm-app's `dev` script and into .devcontainer/Makefile's run-ui/build-ui targets. - Update AGENTS.md guides and CONTRIBUTING docs to match the new layout. Out of scope for this change (flagged as required follow-up, see PR description): .github/workflows/dashboards.yml and ui.yml, and build/packages/rpm/server/SPECS/percona-dashboards.spec still reference the old paths and need a dedicated PR validated with a real CI run and RPM build.
Follow-up to the previous commit: these two docs still pointed at the retired make run-qan-ui/build-qan-ui targets and the old dashboards/pmm-app path.
1 task
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5720 +/- ##
==========================================
+ Coverage 43.59% 45.39% +1.80%
==========================================
Files 415 418 +3
Lines 43134 43334 +200
==========================================
+ Hits 18804 19672 +868
+ Misses 22454 21721 -733
- Partials 1876 1941 +65
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…move Follow-up to the dashboards/pmm-app restructuring: wires up the three pieces that were previously deferred because they needed a real CI run or RPM build to validate. - .github/workflows/dashboards.yml: drop the build/tests jobs (pmm-app lint/test/build is now covered by ui.yml via Turborepo), fix the changed-dashboard-files path pattern for the flattened dashboards/ tree, and repoint the Slack notification jobs at the check job. - .github/workflows/ui.yml: stop ignoring dashboards/** changes (a deleted/renamed dashboard JSON can break plugin.json includes), and add the plugin.json path verification step, fixed to check paths directly against the repo root instead of double-prefixing them with dashboards/. - build/packages/rpm/server/SPECS/percona-dashboards.spec: build ui/apps/pmm-app via turbo instead of the now-gone dashboards/Makefile, and package its dist/ output from the new location. %license/%doc keep pointing at dashboards/LICENSE and dashboards/README.md, which were deliberately kept there for this reason. - .github/CODEOWNERS: give ui/apps/pmm-app/ its own entry so QAN review routing isn't silently absorbed by the generic /ui/ rule.
pmm-app's code (and the two AGENTS.md docs edited when it moved into ui/) had never been run through the workspace's shared Prettier config after joining ui/apps/*, so `make format-check` failed in CI. Ran `prettier --write` with the same glob the check uses; no logic changes, only formatting (mostly curried-function line breaks and operator placement, where Prettier 3.x differs from the 2.x pmm-app was previously formatted with).
The previous formatting fix was run with prettier 3.5.3, matching package.json's ^3.5.3 range but not the version ui/yarn.lock actually resolves it to (3.8.1) — markdown formatting differs slightly between those versions for AGENTS.md and CODE-STYLE.md.
Airbnb's ESLint config (which pmm-app extends) enables several stylistic rules (indent, operator-linebreak, implicit-arrow-linebreak, object-curly-newline, function-paren-newline, no-confusing-arrow, react/jsx-wrap-multilines, react/jsx-one-expression-per-line) that duplicate what Prettier already formats. These previously happened not to conflict because pmm-app's code had been formatted with an older Prettier; reformatting it with the workspace's pinned Prettier (3.8.1, see prior commits) exposed the conflict as 266 lint errors. eslint-config-prettier was already a devDependency but was never actually added to `extends` — add it (and its `prettier/react` companion, which disables the JSX-specific rules) last, so it wins. Also add `avoidEscape` to the `quotes` rule: Prettier intentionally uses double quotes for a string containing an apostrophe to avoid escaping it, which the plain `quotes: single` rule doesn't tolerate. Verified by running the actual pinned ESLint/plugin versions against every file the CI lint step had flagged: 0 errors remain.
- jest.config.js: resolve d3's dist bundle via its actual installed location instead of a hardcoded <rootDir>/node_modules path, which broke once d3 got hoisted to the workspace root. - ui/package.json: dedupe @types/jest (a wildcard "*" request from @types/testing-library__jest-dom was resolving a different, newer major version than pmm-app's own ^29.5.0), and pin react/react-dom consistently across the workspace. - pmm-app/package.json: bump @types/react and @types/react-dom to 18.x to match the actual React 18 runtime instead of the stale 16.x types left over from before the move. - pmm-app/tsconfig.json: widen typeRoots to also include the workspace-hoisted node_modules/@types, since pmm-app no longer has its own fully-populated local @types folder. - Modal.tsx: add the now-required explicit `children` prop (newer @types/react no longer implies it on FC). - Add types/react-table-config.d.ts: react-table's plugin hooks (e.g. useRowSelect) add instance properties TypeScript doesn't know about without this augmentation. - Update a stale Ant Design snapshot (adds an aria-describedby attribute from an upgraded tooltip dependency). Verified locally: format, lint, full test suite, and the full workspace build (pmm, pmm-compat, pmm-app) all pass clean.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket number: PMM-15028
Feature build: SUBMODULES-0
Summary
Developer-experience restructuring requested in PMM-15028:
dashboards/:dashboards/dashboards/*is moved up todashboards/*, so the top-leveldashboards/folder holds only JSON dashboard definitions (plusmisc/tooling andLICENSE/README.md, kept for the RPM spec's%license/%doclines).pmm-appinto theui/monorepo:dashboards/pmm-app→ui/apps/pmm-app, now a Yarn workspace + Turborepo member alongsidepmmandpmm-compat. The plugin id (pmm-app) and its dist layout are unchanged, so no downstream Grafana paths change.src/dashboardsis recreated at its new location, retargeted to the flattened top-leveldashboards/folder.plugin.json's 68 dashboardincludespaths resolve unchanged — verified against the new symlink target.ui/apps/pmm-app/scripts/watch-dashboards.mjs) that mirrors dashboard JSON edits into the provisioned dashboards directory during local dev; wired intopmm-app'sdevscript (concurrentlyalongside the existing webpack watcher) and into.devcontainer/Makefile'srun-ui/build-uitargets. QAN JS/TS changes already auto-reload via the existing webpack + livereload setup, just relocated.pmm-app'slint/testscripts to be non-mutating/one-shot (previouslylintraneslint --fixandtestranjest --watch), matchingpmm-compat's conventions — required for these to behave correctly once folded into the shared workspace pipeline.AGENTS.mdguides (root,dashboards/,ui/,ui/apps/pmm-app/) andCONTRIBUTING.mddocs to match the new layout.CI and packaging
.github/workflows/dashboards.yml: dropped thebuild/testsjobs (now redundant —pmm-applint/test/build is covered byui.ymlvia Turborepo); fixed the changed-dashboard-files path pattern for the flattened tree; repointed the Slack notification jobs at thecheckjob..github/workflows/ui.yml: stopped ignoringdashboards/**changes (a renamed/deleted dashboard JSON can breakplugin.jsonincludes) and moved the "verify plugin.json dashboard paths exist" check here, fixed so it checks paths directly against the repo root instead of double-prefixing withdashboards/.build/packages/rpm/server/SPECS/percona-dashboards.spec:%build/%installnow build and packageui/apps/pmm-appviaturbo run build --filter=pmm-appinstead of the now-removeddashboards/Makefile.%license/%docare untouched (stilldashboards/LICENSE/dashboards/README.md), which is exactly why those two files were kept at the flatteneddashboards/root instead of moving with the rest of the plugin..github/CODEOWNERS: added an explicit/ui/apps/pmm-app/entry so QAN review routing isn't silently absorbed by the generic/ui/rule.Verification done
readlink -f ui/apps/pmm-app/src/dashboardsresolves to the repo-rootdashboards/folder.Verified all 68
plugin.jsoninclude paths resolve through the new symlink (no missing files) — same check the newui.ymlstep runs in CI.YAML-linted both edited workflow files (
yaml.safe_load); reviewed the RPM spec diff to confirm%files/%license/%docare untouched and only%build/%install/%changelogchanged.Grepped the repo for stale
dashboards/dashboards/,dashboards/pmm-app/, ordashboards/Makefilereferences — none found anywhere, including the CI/RPM files.Not verified in this sandbox (no RPM build tooling, no live GitHub Actions, and an organization egress policy blocks
codeload.github.comneeded for a pre-existing transitive git dependency of@grafana/ui): an actualyarn installatui/to confirmpmm-app's dependency hoisting resolves cleanly (it currently pins olderreact/typescript/@grafana/eslint-configthan the rest ofui/), an actual RPM build ofpercona-dashboards.spec, and a live run of the updated GitHub Actions workflows. These should be confirmed by CI once pushed, and ideally with a real RPM build before merging to a release-affecting branch.API Docs updated — N/A, no API changes.
Links to related work items: PMM-15028