feat: added new brave dashboard#97
Conversation
JasperS20
requested review from
FreakyWizard,
Rovasch,
ShunLuk,
SimonvanWijhe,
WybeBosch,
Yannicvanveen,
YvetteNikolov,
Copilot,
dtakken,
ictbeheer,
laravdiemen,
mvdhoek1 and
rivanuff
and removed request for
Copilot
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward dependency management: new private Composer repos, swapping plugin/yard-dashboard suggest for yard/brave-dashboard, adding yard/brave-private-setup as a dev dependency, and routine package version bumps.
Extended reasoning...
Overview
The PR modifies composer.json and composer.lock only. Changes include: adding two private Composer repository sources (satis.yard.nl and packagist.yard.nl/satispress/), moving plugin/yard-dashboard out of suggest in favour of yard/brave-dashboard, adding yard/brave-private-setup ^1.0 to require-dev, and a large set of routine minor/patch version bumps (symfony polyfills, composer tools, yard/* packages, etc.).
Security risks
The new yard/brave-private-setup package includes post-autoload-dump sed scripts that rename the __() helper in a few vendor files to avoid conflicts between WordPress and Laravel. This is a well-known pattern in Bedrock/Acorn projects and poses no security risk. Both new repository sources (satis.yard.nl, packagist.yard.nl) are internal organisation registries, consistent with existing patterns in this repo. The package is dev-only and does not affect production.
Level of scrutiny
Low — all changes are confined to dependency declarations and the lock file. No application code, routing, auth, or data handling is touched. The new private repos and dev dependency follow established patterns already present in the codebase.
Other factors
No bugs were found by the automated bug-hunting system. The PR is small in scope (2 files, all dependency plumbing) and the post-autoload-dump scripts ship as part of a versioned, tagged package rather than being introduced directly into this repo.
JasperS20
force-pushed
the
feat/add-brave-dashboard
branch
from
12f3c46 to
b95a0b9
Compare
Composer package changes
|
There was a problem hiding this comment.
Pull request overview
Updates Brave’s Composer configuration to support Yard private packages and bootstrap tooling, aligning the template’s dependency setup with internal package distribution.
Changes:
- Add Yard internal Composer repositories (
satispress,satis) tocomposer.json. - Add
yard/brave-private-setupas a dev dependency and updatecomposer.lockaccordingly. - Bump locked versions for
yard/brave-scaffoldandcomposer/composer, and add ayard/brave-dashboardsuggestion.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| composer.json | Adds internal repositories, adds yard/brave-private-setup to require-dev, and updates suggest entries. |
| composer.lock | Refreshes lockfile (new dev package + version bumps). |
| "yard/wp-deployer": "dev", | ||
| "yard/brave-dashboard": "" |
There was a problem hiding this comment.
PR title mentions adding a new Brave dashboard, but this change only adds yard/brave-dashboard under suggest (and it isn’t added to require/require-dev or composer.lock). If the dashboard is meant to be included by default, it should be added as a dependency; otherwise consider updating the PR title/description to match the actual change.
| "url": "https://packagist.yard.nl/satispress/" | ||
| }, | ||
| { | ||
| "name": "satis", | ||
| "type": "composer", | ||
| "url": "https://satis.yard.nl" |
There was a problem hiding this comment.
The new internal Composer repositories are unrestricted, so they could potentially provide/override any package name (supply-chain risk). Consider adding an only allowlist (e.g., yard/*, plugin/*, etc.) and/or marking the repo canonical: false if you want Packagist to remain authoritative for public packages.
| "url": "https://packagist.yard.nl/satispress/" | |
| }, | |
| { | |
| "name": "satis", | |
| "type": "composer", | |
| "url": "https://satis.yard.nl" | |
| "url": "https://packagist.yard.nl/satispress/", | |
| "canonical": false | |
| }, | |
| { | |
| "name": "satis", | |
| "type": "composer", | |
| "url": "https://satis.yard.nl", | |
| "canonical": false |
| { | ||
| "name": "satispress", | ||
| "type": "composer", | ||
| "url": "https://packagist.yard.nl/satispress/" | ||
| }, | ||
| { | ||
| "name": "satis", | ||
| "type": "composer", | ||
| "url": "https://satis.yard.nl" | ||
| }, |
There was a problem hiding this comment.
Now that satis/satispress are committed in repositories, the private-setup script still runs composer repo add ... for the same repos (and also composer require yard/brave-private-setup --dev). This can lead to duplicate repository entries and makes the script non-idempotent; consider removing those steps or updating the script to detect existing repos/dependencies before adding them.
| { | ||
| "name": "satispress", | ||
| "type": "composer", | ||
| "url": "https://packagist.yard.nl/satispress/" | ||
| }, | ||
| { | ||
| "name": "satis", | ||
| "type": "composer", | ||
| "url": "https://satis.yard.nl" | ||
| }, |
There was a problem hiding this comment.
Deze twee keys morgen weer weg. Die worden toegevoegd via het private-setup script. Na deze change akkoord 🎉
There was a problem hiding this comment.
In main zijn deze private repos nu wel toegeveogd.
|
Rebase nog even met main om de laatste changes mee te krijgen: https://github.com/yardinternet/brave/pull/83/changes de private setup is wat versimpeld |
JasperS20
force-pushed
the
feat/add-brave-dashboard
branch
from
9b8d2d1 to
73358f6
Compare
No description provided.