Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #48 +/- ##
===========================================
+ Coverage 34.83% 55.24% +20.41%
===========================================
Files 14 20 +6
Lines 1134 1421 +287
===========================================
+ Hits 395 785 +390
+ Misses 739 636 -103 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request adds a CI/CD dashboard feature to the Stand-Up Timer application, allowing teams to monitor the status of GitHub Actions and Jenkins workflows during their daily standup meetings. The dashboard integrates seamlessly into the existing pre-standup flow, providing workflow status visibility before the timer begins.
Changes:
- Adds a CI dashboard screen with support for GitHub Actions and Jenkins workflows
- Implements configuration file support via
~/.config/standup-timer/workflows.yaml - Modifies the pre-standup flow to include navigation between comic screen and dashboard screen
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/services/ci_provider.dart | Defines provider-agnostic interfaces for CI/CD integrations |
| lib/services/github_provider.dart | Implements GitHub Actions API integration |
| lib/services/jenkins_provider.dart | Implements Jenkins API integration |
| lib/services/config_service.dart | Handles YAML configuration file parsing |
| lib/providers/workflows_provider.dart | Manages workflow state with automatic 60-second refresh |
| lib/screens/dashboard_screen.dart | Displays CI workflow statuses with clickable links to runs |
| lib/widgets/timer_section.dart | Adds dashboard navigation to pre-start view |
| lib/widgets/current_speaker.dart | Supports dashboard mode display |
| lib/comic.dart | Simplified to show navigation button to dashboard |
| test/dashboard_screen_test.dart | Comprehensive widget tests for dashboard screen |
| test/timer_section_test.dart | Tests for pre-start flow including dashboard integration |
| test/current_speaker_test.dart | Tests for dashboard mode in current speaker widget |
| pubspec.yaml | Adds yaml and url_launcher dependencies |
| snap/snapcraft.yaml | Adds home directory access for config file reading |
| linux/flutter/generated_plugins.cmake | Registers url_launcher Linux plugin |
| linux/flutter/generated_plugin_registrant.cc | Registers url_launcher native code |
| README.md | Documents CI/CD dashboard configuration |
Comments suppressed due to low confidence (3)
test/timer_section_test.dart:100
- Missing test coverage for the navigation flow from ComicScreen to DashboardScreen. The tests verify that ComicScreen is shown initially and that DashboardScreen is not shown, but there's no test verifying that clicking the "View CI Dashboard" button successfully navigates to the DashboardScreen. Consider adding a test that taps the button and verifies the screen transition.
group('TimerSection pre-start flow', () {
testWidgets('shows ComicScreen in initial state with no participants',
(tester) async {
await tester.binding.setSurfaceSize(const Size(1200, 800));
await tester.pumpWidget(_wrap(_section()));
await tester.pump();
expect(find.byType(ComicScreen), findsOneWidget);
expect(find.byType(DashboardScreen), findsNothing);
});
testWidgets('shows ComicScreen in initial state with participants',
(tester) async {
await tester.binding.setSurfaceSize(const Size(1200, 800));
await tester.pumpWidget(_wrap(_section(people: ['Alice', 'Bob'])));
await tester.pump();
expect(find.byType(ComicScreen), findsOneWidget);
expect(find.byType(DashboardScreen), findsNothing);
});
testWidgets('shows timer UI (not ComicScreen) when timer is running',
(tester) async {
await tester.binding.setSurfaceSize(const Size(1200, 800));
await tester.pumpWidget(_wrap(_section(
people: ['Alice', 'Bob'],
isRunning: true,
currentTime: 60, // mid-run
)));
await tester.pump();
expect(find.byType(ComicScreen), findsNothing);
expect(find.byType(DashboardScreen), findsNothing);
});
testWidgets('shows ComicScreen again after timer returns to initial state',
(tester) async {
await tester.binding.setSurfaceSize(const Size(1200, 800));
// Pump with timer running (non-initial state).
await tester.pumpWidget(_wrap(_section(
people: ['Alice', 'Bob'],
isRunning: true,
currentTime: 60,
)));
await tester.pump();
expect(find.byType(ComicScreen), findsNothing);
// Reset to initial state.
await tester.pumpWidget(_wrap(_section(
people: ['Alice', 'Bob'],
isRunning: false,
currentTime: 120, // back to full duration
)));
await tester.pump();
expect(find.byType(ComicScreen), findsOneWidget);
});
});
lib/providers/workflows_provider.dart:87
- Potential race condition with concurrent refresh calls. If the user manually clicks refresh while a periodic refresh is already in progress, or if multiple refresh calls happen in quick succession, both will run concurrently and the last one to complete will overwrite the results. Consider adding a flag to track if a refresh is in progress and skip concurrent refresh attempts.
Future<void> refresh() async {
if (_providers == null) return;
state = state.copyWith(isLoading: true);
final runs = await Future.wait(
_providers!.map((p) => p.fetchLatestRun()),
);
state = state.copyWith(
runs: runs,
isLoading: false,
lastFetched: DateTime.now(),
);
}
lib/services/config_service.dart:34
- Platform-specific path handling issue. The configPath uses Platform.environment['HOME'] which works on Linux/macOS but may not work correctly on Windows (which uses USERPROFILE). While this app appears to be Linux-focused based on the snap packaging, consider using a more cross-platform approach or documenting the Linux-only limitation.
static String get configPath {
final home = Platform.environment['HOME'] ?? '';
return '$home/.config/standup-timer/workflows.yaml';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This reverts commit da34e0e.
fernando79513
left a comment
There was a problem hiding this comment.
I have installed the snap and tested it locally.
After configuring the yaml file properly, Evertything seems to be working as expected.
LGTM +1!
This PR adds a CI dashboard page, used to monitor together every morning the status of important workflows.
Supports:
Disclaimer: claude is my friend.