refactor!: remove the ELBOW enum #801
Conversation
Introduce the `ElbowValue` type. BREAKING CHANGES: the ELBOW enum has been removed. Use the `ElbowValue` type instead.
|
Warning Rate limit exceeded@tbouffard has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 4 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis change removes the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CellStateStyle
participant ElbowEdgeHandler
participant Elbow
User->>CellStateStyle: Set elbow property (ElbowValue)
ElbowEdgeHandler->>CellStateStyle: Read elbow property
ElbowEdgeHandler->>ElbowEdgeHandler: Compare elbow to 'vertical' or 'horizontal'
Elbow->>CellStateStyle: Access elbow property
Elbow->>Elbow: Compare elbow to 'vertical'
Possibly related PRs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)packages/core/src/types.ts(2 hunks)packages/core/src/util/Constants.ts(0 hunks)packages/core/src/view/handler/ElbowEdgeHandler.ts(2 hunks)packages/core/src/view/mixins/EdgeMixin.type.ts(1 hunks)packages/core/src/view/style/edge/Elbow.ts(1 hunks)packages/html/stories/Indicators.stories.js(2 hunks)
💤 Files with no reviewable changes (1)
- packages/core/src/util/Constants.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build (windows-2022)
- GitHub Check: build (macos-14)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build
🔇 Additional comments (8)
packages/core/src/view/style/edge/Elbow.ts (1)
69-69: String literal replacement looks good.The replacement of
ELBOW.VERTICALenum with the string literal'vertical'aligns with the PR objective to remove the ELBOW enum in favor of the newElbowValuetype.packages/html/stories/Indicators.stories.js (2)
18-18: Import statement correctly updated.The removal of the
constantsimport aligns with the PR objective of removing the ELBOW enum.
61-61: String literal replacement looks good.The replacement of
constants.ELBOW.VERTICALwith the string literal'vertical'maintains the same behavior while removing the dependency on the enum.CHANGELOG.md (1)
19-19: Breaking change properly documented.The changelog has been correctly updated to document the breaking change from
constants.ELBOWtoElbowValue, providing clear migration guidance for users.packages/core/src/view/handler/ElbowEdgeHandler.ts (2)
20-20: Import statement correctly updated.The import statement has been updated to only import
CURSORfrom Constants, removing theELBOWenum import which is being removed from the codebase.
116-116: String literal replacement looks good.The replacement of
ELBOW.VERTICALenum with the string literal'vertical'maintains the same behavior while aligning with the PR objective to remove the enum.packages/core/src/types.ts (2)
1471-1474: Good addition of the ElbowValue type alias.Adding the type alias to replace the enum is a clean approach and aligns with TypeScript best practices for string literal types.
208-208: Appropriate type update for elbow property.The property now uses the newly defined ElbowValue type alias instead of an inline union type, maintaining consistency throughout the codebase.
This simplifies the maintenance of the GH workflow and let easily built all examples locally with a single command.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
build-all-examples.bash (1)
7-15: Handle unmatched globs and consider parallel builds
By default, if no directories match the glob, the loop will iterate once with the literal pattern. You may want to enablenullglobor add an explicit check to avoid silent skips. Additionally, for faster CI runs, you could parallelize builds (e.g., withxargs -Por GNUparallel).Example diff to enable
nullglob:#!/usr/bin/env bash -set -euo pipefail +set -euo pipefail +shopt -s nullglob
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build.yml(1 hunks)build-all-examples.bash(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/build.yml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (windows-2022)
🔇 Additional comments (3)
build-all-examples.bash (3)
1-2: Robust shell setup with strict mode
Using#!/usr/bin/env bashcombined withset -euo pipefailis excellent for catching errors early and avoiding unbound variables.
4-5: Clear startup messaging
The comment and initialechoclearly communicate the script’s purpose to users.
17-18: Confirm overall success
The finalechoprovides clear feedback once all builds complete.
|
Introduce the
ElbowValuetype.Also add a script to build all examples.
This simplifies the maintenance of the GH workflow and let easily built all examples locally with a single command.
BREAKING CHANGES: the ELBOW enum has been removed. Use the
ElbowValuetype instead.Notes
Covers #378
Impact on the size of the examples
Summary by CodeRabbit
Summary by CodeRabbit
Breaking Changes
constants.ELBOW, update your code to use the string values'horizontal'or'vertical'directly.Documentation
Style
Chores