-
Notifications
You must be signed in to change notification settings - Fork 149
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 support for Groups #47
Conversation
Rebase of #40 |
I noticed a typo/spelling error with |
I've update the spelling on |
With the last release, we made new aws-icons-for-plantuml/scripts/icon-builder.py Lines 357 to 358 in 07a49ce
Can you also add the Color updates you made in Feel free to use defaults of these, and only config overrides in a data structure similar to Group:
BorderStyle: Plain
BorderColor: SquidInk
BackgroundColor: White |
@hakanson I've made some updates to the code around |
@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 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.
|
It was user error specifying branch name :( - the command below will push to the properly named
|
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.