Skip to content
This repository has been archived by the owner on Feb 14, 2024. It is now read-only.

docs (modules/vnet): Fix variable description #236

Merged
merged 3 commits into from Feb 21, 2023
Merged

docs (modules/vnet): Fix variable description #236

merged 3 commits into from Feb 21, 2023

Conversation

Lostmaniac9
Copy link
Contributor

Description

Changed id in Variables.tf from network_security_group_id to network_security_group

Motivation and Context

#232

How Has This Been Tested?

I ran the pre-commit tests to what I understand to be the standards of the Contributing document.

Types of changes

Bug fix

Checklist

  • [ x ] I have updated the documentation accordingly.
  • [ x ] I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

I am slightly unsure if I ran the pre-commit script correctly since I don't entirely understand what it does, but I believe it ran correctly and didn't have any significant issues to report.

Changed `network_security_group_id` to `network_security_group`
@Lostmaniac9 Lostmaniac9 requested a review from a team as a code owner February 15, 2023 17:49
@welcome-to-palo-alto-networks

🎉 Thanks for opening this pull request! We really appreciate contributors like you! 🙌

@FoSix
Copy link
Contributor

FoSix commented Feb 16, 2023

@Lostmaniac9 - Thanks for opening this PR. Unfortunately we cannot merge it in this form, just two small steps to do before we can:

  1. please take a look at the Contributing.md document and make sure you run pre-commit in order to update the REDME files
  2. please follow Conventional Commits when titling the PR.

@migara migara changed the title fixes issue #232 docs (modules/vnet): Fix variable description Feb 16, 2023
@Lostmaniac9
Copy link
Contributor Author

Lostmaniac9 commented Feb 16, 2023

I am a bit confused with how to proceed with this. I have been trying to run pre-commit to update the README files but it keep failing when trying to validate terraform docs and tflint and gives an exit code of 127. Any reading I have done in the Contributing.md document hasn't been able to help me. I think I haven't installed tflint and terraform correctly but I'm not sure what I am missing.

What's more is that it looks like above my comment several checks were run for my commit that all worked out so now I am wondering if anything is wrong at all.

@migara
Copy link
Member

migara commented Feb 17, 2023

@Lostmaniac9 can you please paste the error you're getting here?

@Lostmaniac9
Copy link
Contributor Author

image
Hoping this image show up well because the formatting in the console isn't very good. I am pretty sure I've just installed terraform and tflint incorrectly but I haven't been able to figure out how.

@migara
Copy link
Member

migara commented Feb 21, 2023

@Lostmaniac9 not sure entirely. Perhaps try to install tflint and Terraform fmt via a package manager like Chocolatey.

I will fix this for this PR

@migara migara merged commit de75928 into PaloAltoNetworks:main Feb 21, 2023
@welcome-to-palo-alto-networks

🎉 Congrats on getting your first pull request merged! We here at Palo Alto Networks are so grateful! ❤️

sebastianczech pushed a commit that referenced this pull request Oct 18, 2023
Co-authored-by: Migara Ekanayake <2110772+migara@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants