Skip to content
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 Transit Gateway Attachment icon, its alias and Internet Gateway alias. #1038

Merged

Conversation

laiminhtrung1997
Copy link
Contributor

No description provided.

@gabriel-tessier
Copy link
Collaborator

Thank you for the PR,
First as stated in the contributing file the icon must be maximum 256px, yours is 48, and also you need to run autogen script.
About adding alias refer to the #update-aliases section of the contributing file.

You can read all the details about here: about the https://github.com/mingrammer/diagrams/blob/master/CONTRIBUTING.md

@gabriel-tessier gabriel-tessier requested review from gabriel-tessier and removed request for gabriel-tessier November 3, 2024 07:56
@gabriel-tessier gabriel-tessier self-assigned this Nov 3, 2024
@laiminhtrung1997 laiminhtrung1997 force-pushed the transit-gateway-attachment branch from b191513 to a621f92 Compare November 4, 2024 02:58
@laiminhtrung1997
Copy link
Contributor Author

Dear @gabriel-tessier,

First, I appreciate you taking the time to review my contribution.

I’ve implemented your recommendations: I updated the image and ran the autogen script. However, it generated a few additional changes. Should I remove these, or would it be better to add a new commit for them? Or should I leave them as is?

Could you please review it again when you have a moment?

Thank you very much!

@gabriel-tessier
Copy link
Collaborator

gabriel-tessier commented Nov 14, 2024

@laiminhtrung1997
Can you remove the files that are not related to your changes?
Other changes are ok.

@laiminhtrung1997 laiminhtrung1997 force-pushed the transit-gateway-attachment branch from a621f92 to 57f392b Compare November 15, 2024 02:49
@laiminhtrung1997
Copy link
Contributor Author

@gabriel-tessier

I removed unrelated files. Please have a look again. Thank you.

Copy link
Collaborator

@tvqphuoc01 tvqphuoc01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tvqphuoc01 tvqphuoc01 merged commit 085c801 into mingrammer:master Nov 19, 2024
7 checks passed
@laiminhtrung1997 laiminhtrung1997 deleted the transit-gateway-attachment branch November 19, 2024 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants