feat(aws): add compute icons#2
Conversation
Add 1 new database icon(s), regenerate classes/docs, stacked on master. Signed-off-by: Roberto Luna-Rojas <roberto.luna.rojas@gmail.com>
Add 28 new compute icon(s), regenerate classes/docs, stacked on master. Signed-off-by: Roberto Luna-Rojas <roberto.luna.rojas@gmail.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (58)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds numerous new AWS compute icon classes (e.g., Bottlerocket, Dcv, EC2DbInstance, ECS/EKS variants, Elastic Container Registry/Service variants, Outposts variants) to compute.py, one new database icon class in database.py, and updates docs/nodes/aws.md to document these additions. ChangesAWS Icon Classes and Documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Related issues: Suggested labels: documentation, enhancement Suggested reviewers: maintainers of diagrams/aws module Poem:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
| class AuroraAmazonRDSInstanceAternate(_Database): | ||
| _icon = "aurora-amazon-rds-instance-aternate.png" | ||
|
|
There was a problem hiding this comment.
AuroraAmazonRDSInstanceAternate ("aternate") is a near-duplicate of the already-present AuroraAmazonRDSInstanceAlternate ("alternate") at line 15. The PNG filename is also missing the 'l': aurora-amazon-rds-instance-aternate.png. Any code that reaches for the "alternate" variant by name will silently use whichever one it happens to import; the correctly-spelled class already covers this icon slot, so this entry is redundant and can cause user confusion.
| class ElasticContainerServiceCopiiotCli(_Compute): | ||
| _icon = "elastic-container-service-copiiot-cli.png" |
There was a problem hiding this comment.
ElasticContainerServiceCopiiotCli contains a doubled 'i' ("Copiiot"). The AWS service is ECS Copilot CLI. Because the class name and the underlying PNG filename (elastic-container-service-copiiot-cli.png) both carry the same misspelling, any downstream user who tries to import this by its correct name (ElasticContainerServiceCopilotCli) will get an ImportError.
| class EcsAnywhere(_Compute): | ||
| _icon = "ecs-anywhere.png" | ||
|
|
||
|
|
||
| class EksAnywhere(_Compute): | ||
| _icon = "eks-anywhere.png" | ||
|
|
||
|
|
||
| class EksDistro(_Compute): | ||
| _icon = "eks-distro.png" |
There was a problem hiding this comment.
EcsAnywhere, EksAnywhere, and EksDistro use abbreviated prefixes (Ecs, Eks) while every other ECS/EKS sub-resource in this module follows the full-name convention (ElasticContainerService*, ElasticKubernetesService*). This is inconsistent with, e.g., ElasticKubernetesServiceEksOnOutposts added in the same PR. Users discovering the API via autocomplete may not find these classes, and the mixed convention makes the module harder to reason about.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| class ElasticContainerRegistryImage(_Compute): | ||
| _icon = "elastic-container-registry-image.png" | ||
|
|
||
|
|
||
| class ElasticContainerRegistryRegistry(_Compute): | ||
| _icon = "elastic-container-registry-registry.png" | ||
|
|
||
|
|
||
| class ElasticContainerRegistry(_Compute): | ||
| _icon = "elastic-container-registry.png" | ||
|
|
There was a problem hiding this comment.
ElasticContainerRegistry* duplicates the existing EC2ContainerRegistry* family without updating the alias
Three new classes — ElasticContainerRegistry, ElasticContainerRegistryImage, ElasticContainerRegistryRegistry — provide icons that are functionally equivalent to the pre-existing EC2ContainerRegistry, EC2ContainerRegistryImage, EC2ContainerRegistryRegistry. The ECR alias at line 295 still maps to EC2ContainerRegistry, so users relying on the alias will get the old class while new docs point to the new one. If the intent was to migrate naming, consider re-pointing the alias and noting the deprecation; if both sets are intentionally kept, a brief comment explaining the distinction would help.
1 participant
Adds the AWS compute architecture icons.
New icon classes:
Closes #1
Summary by CodeRabbit