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

update formatting #5207

Closed
wants to merge 3 commits into from
Closed

update formatting #5207

wants to merge 3 commits into from

Conversation

pradeepbhadani
Copy link
Contributor

@ghost ghost added the size/s label Dec 17, 2019
@ghost ghost requested a review from megan07 December 17, 2019 09:26
@ghost ghost added the documentation label Dec 17, 2019
@pradeepbhadani
Copy link
Contributor Author

@paddycarver This is the new PR replacing #5097

@pradeepbhadani
Copy link
Contributor Author

@danawillow

@danawillow
Copy link
Contributor

hashicorp gets the full two weeks of christmas / new years off, so paddy won't be back until next week. I'm happy to take care of it this time, but I'd prefer that tagging me into PRs doesn't become a habit :)

Copy link
Contributor

@danawillow danawillow left a comment

Choose a reason for hiding this comment

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

Looks like the main change in this is to an autogenerated resource (you can tell by the comment at the top of the file). The way we add examples for these is pretty different than just editing the file, so it'll be a lot easier for us if you could make the PR against magic modules itself. GoogleCloudPlatform/magic-modules#2869 is a recent PR that added a new example, so you can base yours off of that.

For handwritten resources and docs, we're still happy to accept PRs in TPG and then we can run a script to upstream them that'll preserve you as the author of the commit, but since the MM change is significantly different for generated resources, the authorship story gets fuzzier and it's easier to just open those there.

@pradeepbhadani
Copy link
Contributor Author

Hi @danawillow , I thought paddy might be busy so tagged you as you are very active on this repo.

@paddycarver suggested removing one commit so I raise this new PR after removing problematic commit. I will wait for @paddycarver to have a look at this PR and comment.

Is there an easy way to identify the files that are managed by the Magic module or hand-written?

@danawillow
Copy link
Contributor

Yeah, I'll let him decide if he wants to try upstreaming it once he's back next week.

You can tell if a file is autogenerated by checking if it has a comment at the top that says

***     AUTO GENERATED CODE    ***    AUTO GENERATED CODE     ***

Even if a file is written by hand, you can still modify it in the magic modules repo if you'd like- those live at https://github.com/GoogleCloudPlatform/magic-modules/tree/master/third_party/terraform. We have some documentation around this in the CONTRIBUTING guide. Hope that helps!

@ghost ghost added size/xs and removed size/s labels Jan 29, 2020
@danawillow
Copy link
Contributor

@pradeepbhadani, I'm sorry again about the delay. This currently looks like it has some merge conflict markers as well as some changes that I believe you did already propose against MM. If there's anything else outstanding re: this PR, please feel free to open a new one!

@danawillow danawillow closed this Mar 16, 2020
@ghost
Copy link

ghost commented Apr 16, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants