-
-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add a default color for Media category #593
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Reviewed everything up to 9fd6513 in 58 seconds
More details
- Looked at
17
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_RrGqEVDCnMyEXEa3
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
src/util/classes.ts
Outdated
@@ -49,6 +49,11 @@ export const defaultCategories: Category[] = [ | |||
{ name: ['Work', 'Video'], rule: { type: 'regex', regex: 'Kdenlive' } }, | |||
{ name: ['Work', 'Audio'], rule: { type: 'regex', regex: 'Audacity' } }, | |||
{ name: ['Work', '3D'], rule: { type: 'regex', regex: 'Blender' } }, | |||
{ | |||
name: ['Media'], | |||
rule: { type: null }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rule
type is set to null
which is not a valid value according to the Rule
interface definition. It should be either 'regex' or 'none'.
rule: { type: null }, | |
rule: { type: 'none' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently there are several instances of null
instead of 'none'
, not sure why these aren't caught in CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try to chase them down and replace with 'none'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find any other instance of null used in the rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #593 +/- ##
=======================================
Coverage 26.13% 26.13%
=======================================
Files 27 27
Lines 1630 1630
Branches 281 286 +5
=======================================
Hits 426 426
Misses 1145 1145
Partials 59 59 ☔ View full report in Codecov by Sentry. |
The color value is a placeholder, should you choose to change it 😄
Summary:
Added default color
#F33
for 'Media' category indefaultCategories
array insrc/util/classes.ts
.Key points:
#F33
for 'Media' category indefaultCategories
array.src/util/classes.ts
to include the color in thedata
field of the 'Media' category object.Generated with ❤️ by ellipsis.dev