Merged
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6 +/- ##
============================================
+ Coverage 71.18% 100.00% +28.81%
============================================
Files 3 2 -1
Lines 59 60 +1
Branches 0 13 +13
============================================
+ Hits 42 60 +18
+ Misses 17 0 -17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
steffansluis
approved these changes
Jun 30, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR converts the repo to TypeScript, restructures source and test files, and upgrades test coverage to 100% while removing legacy JavaScript and makefile configurations. Key changes include:
- Conversion of all core code and tests from JavaScript to TypeScript.
- Removal of legacy build scripts and makefiles.
- Updated package configuration and dependency management (including Dependabot).
Reviewed Changes
Copilot reviewed 28 out of 30 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/strategy.test.ts | New TypeScript tests for strategy functionality |
| test/strategy.test.js | Removed legacy JS tests |
| test/strategy.profile.test.ts | New TS tests for profile parsing and error handling |
| test/strategy.profile.test.js | Removed legacy JS profile tests |
| test/strategy.oauth.test.ts | New TS tests for OAuth configuration and token retrieval |
| test/strategy.integration.test.ts | New integration tests for Passport strategy usage |
| test/profile.test.js | Removed legacy JS profile tests |
| test/package.test.js | Removed legacy package tests |
| test/data/* | New test data files for profile examples |
| support/mk/* | Removed legacy makefiles and build scripts |
| src/strategy.ts | Rewritten Brightspace strategy in TypeScript |
| src/index.ts | Updated entry point to export the new TypeScript strategy |
| package.json | Updated version, script commands, and dependency management |
| lib/* | Removed legacy JavaScript implementation files |
| eslint.config.mjs | Added new ESLint configuration tailored for TypeScript projects |
| README.md | Updated instructions and installation steps |
| Makefile | Removed legacy makefile |
| .nycrc.json | Updated NYC configuration for TypeScript code coverage |
| .github/workflows/node.yml | Added build step in GitHub Actions workflow |
| .github/.dependabot.yml | Added Dependabot configuration |
Comments suppressed due to low confidence (1)
test/strategy.test.ts:1
- [nitpick] Review the necessity of the @ts-nocheck comment; removing it could help catch type errors in tests and improve overall type safety.
// @ts-nocheck
|
|
||
| this.options = options; | ||
|
|
||
| console.log(`Configuring strategy with options:`, this.options); |
There was a problem hiding this comment.
[nitpick] Consider removing or replacing the console.log statement with a proper logging framework for production use to avoid exposing sensitive configuration data.
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
2 participants
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.
Typescriptify the repo and upgrade coverage to a 100%
Changes:
[sc-62619]