Skip to content

Comments

fix: remove technical debt comments and implement breaking changes#12

Open
devin-ai-integration[bot] wants to merge 4 commits intomainfrom
devin/1768244985-cleanup-technical-debt
Open

fix: remove technical debt comments and implement breaking changes#12
devin-ai-integration[bot] wants to merge 4 commits intomainfrom
devin/1768244985-cleanup-technical-debt

Conversation

@devin-ai-integration
Copy link

@devin-ai-integration devin-ai-integration bot commented Jan 12, 2026

Summary

This PR addresses technical debt by implementing breaking changes that were previously marked with TODO comments. The changes include:

  1. Date Service Format Aliases: Updated format aliases from shorthand notation (e.g., yMdjm) to explicit formats (e.g., M/d/yy, h:mm a) as indicated by TODO comments. This affects short, medium, shortDate, mediumDate, longDate, fullDate, shortTime, and mediumTime formats.

  2. Sort Item Component: Changed EventEmitter<any> to EventEmitter<void> since the emit() call doesn't pass any value.

  3. Flyout Instance: Made the constructor parameter componentInstance required instead of optional, removing the deprecation warning.

  4. Repeater Service: Removed an outdated TODO comment (the type was already SkyRepeaterExpandModeType, not including string).

Updates since last revision

  • Updated test expectations in date.pipe.spec.ts and date.service.spec.ts to match new format output:
    • Changed from 4-digit years (1/1/2000) to 2-digit years (1/1/00)
    • Changed French locale expectations from 2000-01-01 00 h 00 to 1/1/00, 12:00 h

Review & Testing Checklist for Human

  • CRITICAL: Verify the 2-digit year format is intentional - dates will now display as 1/1/00 instead of 1/1/2000 for all consumers using short, shortDate, etc.
  • CRITICAL: Verify the French locale format change is correct - changed from 2000-01-01 00 h 00 to 1/1/00, 12:00 h (this is a significant change in locale-specific formatting)
  • Check if any internal or external code depends on the 4-digit year format for parsing or display
  • Verify the EventEmitter<void> change doesn't break consumers that might be subscribing and expecting a value
  • Check if any code creates SkyFlyoutInstance without passing componentInstance - this will now be a compile error

Recommended Test Plan:

  1. Wait for CI tests to complete and verify all pass
  2. Test date formatting in a consuming application with both en-US and fr-CA locales to verify the new formats render correctly
  3. Search codebase for new SkyFlyoutInstance to ensure all usages pass the required parameter
  4. Verify any date parsing logic doesn't depend on the old 4-digit year format

Notes

  • TypeScript compilation was verified locally for affected libraries (datetime, flyout, lists)
  • Lint checks passed
  • Test expectations were updated to match the new format output - human should verify these expected values are correct
  • Some CI checks (risk labels, conventional-title) are failing due to PR metadata, not code issues

Link to Devin run: https://app.devin.ai/sessions/e3d3658a90434b37865e9044c2af4e60
Requested by: @bcmake

- Update date service format aliases to use explicit formats instead of shorthand notation
- Change EventEmitter<any> to EventEmitter<void> in sort-item component
- Make flyout instance constructor parameter required (remove optional)
- Remove outdated TODO comment from repeater service
- Update date service tests to match new format aliases

Co-Authored-By: benc@cognition.ai <Benc@windsurf.com>
@devin-ai-integration
Copy link
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Co-Authored-By: benc@cognition.ai <Benc@windsurf.com>
Co-Authored-By: benc@cognition.ai <Benc@windsurf.com>
- Update date.pipe.spec.ts French locale tests to expect '12:00 h' format
- Update date.service.spec.ts to expect 2-digit years from 'M/d/yy' format
- Update date.service.spec.ts French locale tests to expect new format

Co-Authored-By: benc@cognition.ai <Benc@windsurf.com>
@devin-ai-integration devin-ai-integration bot deployed to e2e-team-members January 12, 2026 20:41 Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants