Closed mcwarman closed 2 years ago
Rebase of #40
I noticed a typo/spelling error with Avalability
- one example is AvalabilityZoneGroup(az_1, "Availability Zone 1") {
but I think there were a couple more.
I've update the spelling on Avalability
.
With the last release, we made new config.yml
generation easier - look for create_config_template()
in icon-builder.py
- can you see if you can also add the new BorderStyle
, BorderColor
, BackgroundColor
subproperties of Group
similar to how Color
is added?
Can you also add the Color updates you made in config.yml
to the embedded template in icon-builder.py
? https://github.com/awslabs/aws-icons-for-plantuml/blob/main/scripts/icon-builder.py#L161 ?
Feel free to use defaults of these, and only config overrides in a data structure similar to GROUPICONS_COLORS
Group:
BorderStyle: Plain
BorderColor: SquidInk
BackgroundColor: White
@hakanson I've made some updates to the code around --create-config-template
and updated how it works slightly to use defaults.
@mcwarman thanks for the update. This is still on my radar, but it might be another week before I can take a look.
Thanks again for this PR. I spent time over the weekend building some diagrams and made some code changes. Can you select the "Allow edits from maintainers" checkbox (see https://github.blog/2016-09-07-improving-collaboration-with-forks/) so I can push these changes into your branch so they will flow back into this PR?
As I was experimenting with example diagrams, I didn't love the macro names for creating the groups (not your fault, since you used the existing icon names). I also noticed some Group icons from the most recent AWS icons deck (like AWS Account) were missing so I extracted those images. I wanted to add default labels so just AWSAccountGroup(account)
could work but still allow label as the second parameter. I created a way to use a zero-length file to trigger creation of the empty groups (Available Zone, Security Group) instead of putting them in AWSGroups.yaml. In the end, there were config overrides for every Group icon, so it ended up being easier to pre-configure all of the Groups category in YAML vs Python code + extra .puml file.
Those were great Groups examples, which I had to update with my renamed macros among other tweaks. I did end up renaming the files to be prefixed with "Groups - " and added a couple more so I could fight with PlantUML rendering. Please let me know what you think of these changes.
@mcwarman have you had a chance to see my comment above? I'd like to get this committed next week, either on this PR, or I can also create my own PR if you don't want to grant access.
@hakanson sorry I missed this, it was already turned on, I've toggled it though.
Maybe user error on my part? I was getting this error message. I'll look at the docs and see what I'm doing wrong.
To https://github.com/mcwarman/aws-icons-for-plantuml.git ! [remote rejected] mcwarman-groups -> mcwarman-groups (permission denied) error: failed to push some refs to 'https://github.com/mcwarman/aws-icons-for-plantuml.git'
It was user error specifying branch name :( - the command below will push to the properly named groups
branch
git push mcwarman mcwarman-groups:groups
~I think its push to origin looking at these docs~:
Actually I've just reread the instructions, I'm not sure, but this might help:
It looks like my commits are appearing on this PR now - take a look and let me know what you think.
Looks good to me.
Reviewed and discussed, ship it!
FYI @jfrconley
@mcwarman - thanks again for the PR. We pushed a release with this yesterday.
Issue #, if available: N/A
Description of changes:
Adds the support for Groups. Some example usage below.
Examples
VPC
Auto Scaling Groups
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.