-
Notifications
You must be signed in to change notification settings - Fork 13
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
Discussion - My terraform support contribution may not work as intended #2
Comments
Craig:
Not sure whether you’ll get this or not, let me know.
I’m not a Terraform guy, so couldn’t really do much of an analysis of your code. Please clean up as much as possible and remerge – anyone who wants to use it has to be pretty deep in the technologies anyway.
Any comments to that effect (“this is not turnkey, you have modifications to do”) would be great!
Thanks, Gary
[Microsoft]
***@***.***
Gary L. Mullen-Schultz
Sr. Cloud Solutions Architect
Customer Success Unit
Healthcare
Office: +1 (952) 832-8011
Mobile: +1 (612) 414-9300
3601 West 76th St., Suite 600
Edina, MN 55435
Want to meet with me? Book time here<https://aka.ms/MeetGary>.
My Certifications: https://www.youracclaim.com/users/gary.lee
From: Craig Thacker ***@***.***>
Sent: Tuesday, September 6, 2022 5:23 PM
To: gamullen/FindNextCIDRRange ***@***.***>
Cc: Subscribed ***@***.***>
Subject: [gamullen/FindNextCIDRRange] Discussion - My terraform support contribution may not work as intended (Issue #2)
Hey Gary 👋
Thanks for merging the PR! To be honest, I didn't actually think it would be merged without some comments from yourself as some of it is abit messy as it does have some requirements from my own GitHub Org.
As it is right now - you may want to revert the merge back to your own last commit since you will get emailed when it fails fail it's CI checks and whatnot since it won't have my organisation secrets, as well as using some custom terraform modules which would be nicer to be generic terraform for a repo like yours.
Although that's up to you 😄
I'll (try) prepare a new PR which will be more suitable for merge into your repo, it's more the fact I wanted to contribute something back as I found your function app hugely useful for something I was looking for in a personal project of mine.
—
Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgamullen%2FFindNextCIDRRange%2Fissues%2F2&data=05%7C01%7CGary.Mullen-Schultz%40microsoft.com%7Cc77bd3ceee234e905d7d08da905662da%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637980997949969998%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=6vF0RNkON3uCoiwuM%2BG0BR7avPPZJRwqCcKm4Ej1qj4%3D&reserved=0>, or unsubscribe<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FALDK3HE4GQHO4YFO2E7SMP3V4675BANCNFSM6AAAAAAQGHOJ6M&data=05%7C01%7CGary.Mullen-Schultz%40microsoft.com%7Cc77bd3ceee234e905d7d08da905662da%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637980997949969998%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Im7rN%2B1UV5a7uewycTY2kX%2BouVUbx61hYWY9qHtLbrE%3D&reserved=0>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.******@***.***>>
|
I have linked a PR for this issue |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hey Gary 👋
Thanks for merging the PR! To be honest, I didn't actually think it would be merged without some comments from yourself as some of it is abit messy as it does have some requirements from my own GitHub Org.
As it is right now - you may want to revert the merge back to your own last commit since you will get emailed when it fails fail it's CI checks and whatnot since it won't have my organisation secrets, as well as using some custom terraform modules which would be nicer to be generic terraform for a repo like yours.
Although that's up to you 😄
I'll (try) prepare a new PR which will be more suitable for merge into your repo, it's more the fact I wanted to contribute something back as I found your function app hugely useful for something I was looking for in a personal project of mine.
The text was updated successfully, but these errors were encountered: