-
Notifications
You must be signed in to change notification settings - Fork 61
feat(core): tip about VRA when a build uses Layout review mode (PER-9502) #2276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4458a37
46efb50
591ce00
e08ba27
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -427,6 +427,21 @@ export function createSnapshotsQueue(percy) { | |
| percy.log.warn(`Build #${build.number} failed: ${build.url}`, { build }); | ||
| await runDoctorOnFailure(percy); | ||
| } else if (build?.id) { | ||
| if (build.layoutUsed) { | ||
| percy.log.warn('Tip: VRA is Percy\'s recommended visual review mode — more accurate and adaptable than Layout. Learn more: https://www.browserstack.com/docs/percy/ai-agents/visual-review-agent/overview.'); | ||
|
|
||
| // instrument the recommendation; telemetry must never fail a build | ||
| try { | ||
| await percy.client.sendBuildEvents(build.id, { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Medium] Telemetry duplicates centralized pattern and omits cliVersion/clientInfo This swallow-and-debug-log telemetry call reimplements the pattern already centralized in Suggestion: Route through a shared telemetry helper, or add Reviewer: stack:code-reviewer
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not applicable here |
||
| message: 'VRA recommendation shown for a build using Layout review mode' | ||
| }, {}, { | ||
| eventName: 'percy_cli_vra_recommendation_emitted', | ||
| category: 'percy:cli' | ||
| }); | ||
| } catch (err) { | ||
| percy.log.debug('VRA recommendation telemetry failed', err); | ||
| } | ||
| } | ||
| await percy.client.finalizeBuild(build.id); | ||
| percy.log.info(`Finalized build #${build.number}: ${build.url}`, { build }); | ||
| } else { | ||
|
|
@@ -444,6 +459,9 @@ export function createSnapshotsQueue(percy) { | |
| .handle('push', (snapshot, existing) => { | ||
| let { name, meta } = snapshot; | ||
|
|
||
| // track layout usage to tip about VRA when the build is finalized | ||
| if (snapshot.enableLayout) build.layoutUsed = true; | ||
|
|
||
| // log immediately when not deferred or dry-running | ||
| if (!percy.deferUploads) percy.log.info(`Snapshot taken: ${snapshotLogName(name, meta)}`, meta); | ||
| if (percy.dryRun) percy.log.info(`Snapshot found: ${snapshotLogName(name, meta)}`, meta); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Low] Tip emitted at WARN level
This is a promotional/informational tip, but it's logged via
percy.log.warn. WARN normally signals something went wrong, and CI log processors or--fail-on-warning-style tooling may surface a benign mode-recommendation as a warning.Suggestion: Use
percy.log.info(or a dedicated tip level) unless WARN-level visibility is an explicit product requirement — in which case a one-line comment noting that intent would prevent future churn.Reviewer: stack:code-reviewer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not applicable, it should be warning,