feature/fp ts #5
Conversation
RyosukeDTomita
had a problem deploying
to
github-pages
GitHub Actions
Failure
There was a problem hiding this comment.
Pull request overview
This pull request refactors the codebase to use the fp-ts functional programming library, restructures the code into modular files, and adds comprehensive unit testing with GitHub Actions CI/CD integration.
Key Changes
- Refactored monolithic
main.tsinto separate modules (src/main.ts,src/streak.ts,src/githubApi.ts,src/config.ts,src/type.ts) - Integrated fp-ts library for functional programming patterns, particularly using
Optiontypes instead of null values - Added unit tests for the streak calculation logic with multiple test cases covering edge scenarios
- Added GitHub Actions workflow for automated Deno testing using Nix environment
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/streak_test.ts | New unit tests for calculateStreak function with 4 test cases covering different streak scenarios |
| src/type.ts | New type definitions module extracting ContributionDay and Week types |
| src/streak.ts | New streak calculation and SVG generation module using fp-ts, with JST timezone support |
| src/main.ts | New main entry point with refactored logic using fp-ts Option types |
| src/githubApi.ts | New GitHub API client module for fetching contribution data |
| src/config.ts | New configuration module for constants and environment variables |
| main.ts | Removed monolithic file in favor of modular structure |
| deno.lock | Added lock file with [email protected] and @std/assert@1 dependencies |
| deno.jsonc | Updated with import maps for fp-ts modules and new test task |
| README.md | Formatting improvements to code block indentation |
| .github/workflows/main.yml | Added push trigger for E2E testing |
| .github/workflows/deno_test.yml | New CI workflow for running Deno tests via Nix |
| .github/copilot-instructions.md | New instructions for using fp-ts Option and Nix Flakes in the project |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - cron: "0 0 * * *" | ||
| workflow_dispatch: | ||
| #push | ||
| pull_request: |
There was a problem hiding this comment.
The comment says "fix main.yml to e2e" but actually adds a "push" trigger with a comment "for E2E". However, enabling push triggers on every push is typically not intended for E2E testing in production workflows. This might cause unnecessary workflow runs. Consider using a more specific branch pattern or removing this after E2E testing is complete.
| const today = new Date().toISOString().slice(0, 10); | ||
| const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000) | ||
| .toISOString() | ||
| .slice(0, 10); | ||
| const twoDaysAgo = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000) | ||
| .toISOString() | ||
| .slice(0, 10); |
There was a problem hiding this comment.
The tests are using UTC date strings (via new Date().toISOString()) but the actual calculateStreak function uses JST dates (via getJSTDateString). This creates a timezone mismatch that could cause the tests to fail or behave unexpectedly, especially when run near midnight. The test dates should be converted to JST to match the implementation.
| const today = new Date().toISOString().slice(0, 10); | ||
| const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000) | ||
| .toISOString() | ||
| .slice(0, 10); | ||
| const twoDaysAgo = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000) | ||
| .toISOString() | ||
| .slice(0, 10); |
There was a problem hiding this comment.
The tests are using UTC date strings (via new Date().toISOString()) but the actual calculateStreak function uses JST dates (via getJSTDateString). This creates a timezone mismatch that could cause the tests to fail or behave unexpectedly, especially when run near midnight. The test dates should be converted to JST to match the implementation.
| const today = new Date().toISOString().slice(0, 10); | ||
| const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000) | ||
| .toISOString() | ||
| .slice(0, 10); | ||
| const twoDaysAgo = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000) | ||
| .toISOString() | ||
| .slice(0, 10); |
There was a problem hiding this comment.
The tests are using UTC date strings (via new Date().toISOString()) but the actual calculateStreak function uses JST dates (via getJSTDateString). This creates a timezone mismatch that could cause the tests to fail or behave unexpectedly, especially when run near midnight. The test dates should be converted to JST to match the implementation.
| const today = new Date().toISOString().slice(0, 10); | ||
| const yesterday = new Date(Date.now() - 24 * 60 * 60 * 1000) | ||
| .toISOString() | ||
| .slice(0, 10); | ||
| const twoDaysAgo = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000) | ||
| .toISOString() | ||
| .slice(0, 10); |
There was a problem hiding this comment.
The tests are using UTC date strings (via new Date().toISOString()) but the actual calculateStreak function uses JST dates (via getJSTDateString). This creates a timezone mismatch that could cause the tests to fail or behave unexpectedly, especially when run near midnight. The test dates should be converted to JST to match the implementation.
| /** | ||
| * @param weeks | ||
| * @returns | ||
| */ |
There was a problem hiding this comment.
| /** | ||
| * @param streakInfo | ||
| * @returns | ||
| */ |
There was a problem hiding this comment.
feat: just run using fp-ts
feat: Add GitHub Actions workflow for Deno tests
feat: add unit test
chore: fix main.yml to e2e
using fp-ts
add unit test