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

Example wordpress #818

Merged
merged 50 commits into from
Oct 7, 2022

Conversation

skalolazka
Copy link
Member

@skalolazka skalolazka commented Sep 15, 2022

This is still work in progress, but almost finished, so the directory structure and logic can be reviewed.

Fixes #781

Natalia Strelkova and others added 23 commits July 18, 2022 14:43
@skalolazka skalolazka linked an issue Sep 15, 2022 that may be closed by this pull request
@ludoo
Copy link
Collaborator

ludoo commented Sep 15, 2022

This is great, let us know once it's ready for review!

@skalolazka
Copy link
Member Author

Thank you!
I think you can actually already review, if that's OK. The things I see as still a TODO are:

  1. Populate the README at the wordpress/ directory level
  2. Add a concrete tag to the Docker image
    This is quite small. I would be glad to receive some feedback on the architecture and style at this point already. Thanks a lot beforehand!

Copy link
Collaborator

@juliocc juliocc left a comment

Choose a reason for hiding this comment

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

Any reason to put this blueprint in blueprints/third-party-solutions/wordpress/cloudrun instead of blueprints/third-party/wordpress directly?

@skalolazka
Copy link
Member Author

Any reason to put this blueprint in blueprints/third-party-solutions/wordpress/cloudrun instead of blueprints/third-party/wordpress directly?

Yes, the original idea was to have another example on VMs. This was discussed with Ludovico. We can move one folder up for now, of course, and change later, once the other example is there. Let us know.

Copy link
Collaborator

@ludoo ludoo left a comment

Choose a reason for hiding this comment

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

couple more comments :)

@skalolazka skalolazka dismissed juliocc’s stale review October 7, 2022 06:38

As discussed, the reasoning is valid, so leaving as is for now.

@skalolazka skalolazka requested a review from ludoo October 7, 2022 06:38
@skalolazka
Copy link
Member Author

I just found a bug, not sure it's related to Terraform, but please don't merge just yet. But otherwise my new changes can be reviewed.

@skalolazka
Copy link
Member Author

Bug not a real bug, added a README point=) Ready to be merged from my side=)

@ludoo
Copy link
Collaborator

ludoo commented Oct 7, 2022

Bug not a real bug, added a README point=) Ready to be merged from my side=)

awesome, and thanks for all the hard work! let us give it a last glance and this can then be merged today!

@skalolazka skalolazka merged commit d33e04c into GoogleCloudPlatform:master Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wordpress on Cloud Run example
5 participants