Feature/hardening and publish #7
Merged
Conversation
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
- Migrate all JavaScript files to TypeScript - Organize code with models/ and services/ structure - Add comprehensive type definitions and interfaces - Implement proper build system with webpack - Move icons from root to src/icons/ - Add CODEOWNERS for security - Add INSTALLATION.md documentation - Update .gitignore to exclude artifacts/ - Remove development utilities and test files - Fix icon loading to match original behavior - Improve settings UI with theater mode detection - Add versioned packaging script All changes maintain exact functionality of original JS version
- Configure Karma with webpack for TypeScript testing - Add karma-webpack to properly bundle tests - Create MockChromeStorage class with repository pattern - Add tests for AspectRatioService (8 tests - all passing) - Add tests for StorageService (5 tests - all passing) - Add tests for TabService (2 tests - all passing) - Total: 15 tests passing UI improvements: - Disable all form controls when theater mode is off - Replace 4:3 aspect ratio with 16:10 (more common for modern monitors) - Fix slider remaining enabled when it should be disabled All tests run in ChromeHeadless with real Chrome APIs available
Keep only essential documentation: - Chrome Web Store permissions and description - Privacy policy setup guide - CI/CD setup guide
There was a problem hiding this comment.
Pull Request Overview
This PR introduces comprehensive documentation, repository ownership, CI/CD automation, and builds a robust foundation for publishing the extension to Chrome and Edge stores. The changes establish best practices for development workflows, user guidance, and compliance requirements.
- Adds complete CI/CD pipeline with automated testing, building, and publishing workflows
- Introduces comprehensive documentation including installation guides, privacy policy, and store submission materials
- Establishes repository ownership and security through CODEOWNERS file
Reviewed Changes
Copilot reviewed 34 out of 44 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
CI pipeline for automated testing, linting, and building on every push/PR |
.github/workflows/cd.yml |
CD pipeline for automated publishing to Chrome/Edge stores on releases |
CODEOWNERS |
Defines code ownership and review requirements for security |
docs/ |
Comprehensive documentation for privacy policy, store descriptions, permissions, CI/CD setup |
PRIVACY.md + PRIVACY.html |
Privacy policy in multiple formats for compliance and store submission |
INSTALLATION.md |
Detailed installation and usage guide for developers and users |
README.md |
Enhanced project documentation with development workflows and architecture |
Comments suppressed due to low confidence (1)
src/services/TabService.spec.ts:1
- Test is using className 'tab' but TabService.setupTabListeners() queries for '.tab-btn'. Both tab elements should use className 'tab-btn' to properly test the service functionality.
import { TabService } from './TabService';
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- Remove build and test steps for now - Focus on validating CI setup works - Will add comprehensive testing in follow-up PR
- Change 'general' to 'general-tab' - Change 'advanced' to 'advanced-tab' - Tests now align with actual DOM structure in popup.html
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 34 out of 44 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- Add automated version bump scripts (major/minor/patch) - Add version sync checker to prevent mismatches - Update both package.json and manifest.json to 1.0.0 - Update README with version bump workflow - Version scripts ensure both files stay synchronized
- Add version sync check to prevent mismatches - Run ESLint on all TypeScript files - Execute full Karma test suite - Build extension and verify artifacts - Upload build artifacts for Node.js 20.x - Matrix testing on Node.js 18.x and 20.x
CI Pipeline (.github/workflows/ci.yml): - Add version-specific artifact uploads (e.g., linkedin-learning-enhancer-v1.0.0) - Package extension as ZIP on every build - Artifacts retained for 90 days, overwrite previous builds of same version - Add 'latest' artifact for main branch builds - Run full validation: lint, test, version check, build Release Workflow (.github/workflows/release.yml): - Create GitHub releases from version tags - Upload ZIP and SHA256 checksum to releases - Full CI validation before release CD Pipeline (.github/workflows/cd.yml): - Manual trigger only (workflow_dispatch) - Store publishing disabled (no credentials yet) - Ready to enable when Chrome/Edge credentials available README Updates: - Add direct download link to GitHub Actions artifacts - Add CI status and release badges - Detailed installation instructions for Chrome and Edge - Document automated release process Users can now download builds directly from GitHub Actions!
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 37 out of 47 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- Rename 'partialSettings' to 'settings' for clarity in content.ts - Add early return in loadSettingsAndApply() to avoid unnecessary async calls - Remove unused imports: AspectRatioService, ChromeMessage - Remove unused variables: buttonCheckInterval, err catch variable - Change console.log to console.warn for consistency - Upgrade ESLint rules from 'warn' to 'error' to prevent deprecated/unused code - Fix TypeScript 'any' types in StorageService.spec.ts with proper Chrome storage types - All 15 tests passing, lint clean
- Replace 'any' with Partial<Settings> in ChromeMessage interface - Replace 'any' with Record<string, unknown> in MockChromeStorage class - Improve type safety across test mocks and models - Maintains 15 passing tests, lint clean
- Create Messages.ts in models/ to define all message type interfaces - Remove duplicate VideoPlayerStateMessage, CheckVideoPlayerMessage from background.ts and content.ts - Remove duplicate SettingsUpdatedMessage from content.ts - Export ContentMessage and BackgroundMessage union types - Both files now import from shared models for consistency - Follows DRY principle and addresses Copilot PR review feedback - All 15 tests passing, lint clean
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 38 out of 48 changed files in this pull request and generated 7 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This pull request introduces comprehensive improvements to the project's developer experience, automation, and security. It adds detailed documentation for installation and distribution, establishes clear code ownership, and implements robust CI/CD workflows for testing, releasing, and publishing the browser extension.
Documentation and Developer Experience:
INSTALLATION.mdguide covering developer setup, project structure, browser loading instructions, packaging, feature overview, troubleshooting, and contribution guidelines.Code Ownership and Security:
CODEOWNERSfile assigning ownership of all files and critical directories to@rmjoia, helping ensure proper code review and preventing unauthorized changes, especially for sensitive files and build outputs.Continuous Integration (CI):
.github/workflows/ci.ymlto automate testing, linting, building, and artifact packaging for multiple Node.js versions on push and pull requests, with artifact uploads for easy access to build outputs.Release Automation:
.github/workflows/release.ymlto automate building, packaging, and releasing extension artifacts on tag pushes or manual triggers. This workflow includes security audits, lints, tests, build verification, release creation, asset uploads, and SHA256 checksum generation for secure distribution.Store Publishing Automation:
.github/workflows/cd.ymlfor optional automated publishing to Chrome Web Store and Edge Add-ons, contingent on the presence of required store credentials in repository secrets.Ratio modes: