Skip to content

Conversation

@klijakub
Copy link
Contributor

Hello,

  • new pipelines subpage ( /pipelines routing)
  • added new assets from the approved design
  • features section

Jira Item: WWW-92

@netlify
Copy link

netlify bot commented Nov 12, 2020

Deploy preview for keen-clarke-470db9 ready!

Built with commit 1eadb48

https://deploy-preview-375--keen-clarke-470db9.netlify.app

@eak12913
Copy link
Contributor

@klijakub I'm not sure if this is just something on my end - but the terraform plan and terraform apply seem like they are much larger than the surrounding text. See the screenshot:

image

I think @josh-padnick will be the primary person to review this - but just stood out to me as a little odd. You may want to make it a bit smaller.

@josh-padnick
Copy link
Contributor

Thanks for submitting this PR! I just submitted a commit with the final set of changes, and this is now ready to merge.

@eak12913 @oredavids Could one of you approve and merge this as I'm now a contributor?

@klijakub There is still one additional change to make. We actually got some extra feedback on the main image. Could you make the following updates to that image?

  • Remove the Lambda function from the diagram
  • Change "CI Server" to "Your CI Server"
  • Add another label to the CI Server called "Pipelines CLI" with the Grunty logo
  • Add another label "Your Terraform Code" to the Terraform logo

josh-padnick
josh-padnick previously approved these changes Nov 18, 2020
@klijakub
Copy link
Contributor Author

klijakub commented Nov 18, 2020

Thanks for submitting this PR! I just submitted a commit with the final set of changes, and this is now ready to merge.

@eak12913 @oredavids Could one of you approve and merge this as I'm now a contributor?

@klijakub There is still one additional change to make. We actually got some extra feedback on the main image. Could you make the following updates to that image?

  • Remove the Lambda function from the diagram
  • Change "CI Server" to "Your CI Server"
  • Add another label to the CI Server called "Pipelines CLI" with the Grunty logo
  • Add another label "Your Terraform Code" to the Terraform logo

@josh-padnick
Thanks for the review! :) Sure I will let know our design team and we will get back to you with the new diagram

Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Just added a few trivial grammar NITs

@klijakub
Copy link
Contributor Author

@brikis98 fixed! :)

@josh-padnick the new diagram is ready and committed:

image

@eak12913
Copy link
Contributor

@klijakub Could you please update your branch and then I can approve it? Otherwise I believe my approval may be dismissed if you push additional commits after I approve.

Thanks and then I'll be able to approve and merge!

@klijakub
Copy link
Contributor Author

@klijakub Could you please update your branch and then I can approve it? Otherwise I believe my approval may be dismissed if you push additional commits after I approve.

Thanks and then I'll be able to approve and merge!

Sorry, didn't notice your comment. Sure, I will do it asap

@josh-padnick
Copy link
Contributor

@klijakub Thanks for the fast turnaround! Ok, just one more minor change. Can you modify "Pipelines CLI" to "Gruntwork Pipelines CLI"? Otherwise, everything looks great.

@josh-padnick
Copy link
Contributor

@eak12913 Could you give me another approval? Had to resolve merge conflicts before I could merge.

eak12913
eak12913 previously approved these changes Nov 18, 2020
Copy link
Contributor

@eak12913 eak12913 left a comment

Choose a reason for hiding this comment

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

Rubber stamping. Thanks!

@josh-padnick
Copy link
Contributor

@oredavids Sorry, looks like I need yet one more non-contributor approval here...not sure why?

@klijakub
Copy link
Contributor Author

@klijakub Thanks for the fast turnaround! Ok, just one more minor change. Can you modify "Pipelines CLI" to "Gruntwork Pipelines CLI"? Otherwise, everything looks great.

Done ;)

@josh-padnick
Copy link
Contributor

Thanks for the fast turnaround! This is ready to merge once we can get one more non-contributor approval.

@josh-padnick
Copy link
Contributor

@eak12913 Could you (a) resolve this merge conflict, and (b) coordinate with another (non-contributor) Grunt to get this deployed?

@eak12913
Copy link
Contributor

I will take a look!

@eak12913
Copy link
Contributor

@josh-padnick @klijakub: I just cloned Altology's fork of this repo and tried to resolve the conflict (which I was able to do) but I can't actually push to this repo.

Pushing to https://github.com/Altalogy/gruntwork-io.github.io.git
remote: Permission to Altalogy/gruntwork-io.github.io.git denied to eak12913.
fatal: unable to access 'https://github.com/Altalogy/gruntwork-io.github.io.git/': The requested URL returned error: 403

The fix is simple - for prism.css just accept what's coming from Gruntwork. I'm not even sure if it needed to change in this PR as we don't appear to be using prism to render any code on the Pipelines page.

@klijakub
Copy link
Contributor Author

@josh-padnick @klijakub: I just cloned Altology's fork of this repo and tried to resolve the conflict (which I was able to do) but I can't actually push to this repo.

Pushing to https://github.com/Altalogy/gruntwork-io.github.io.git
remote: Permission to Altalogy/gruntwork-io.github.io.git denied to eak12913.
fatal: unable to access 'https://github.com/Altalogy/gruntwork-io.github.io.git/': The requested URL returned error: 403

The fix is simple - for prism.css just accept what's coming from Gruntwork. I'm not even sure if it needed to change in this PR as we don't appear to be using prism to render any code on the Pipelines page.

Oh sorry, @eak12913 , you weren't a member of the forked repo, I sent you an invite, please let me know if you're able now?

@eak12913
Copy link
Contributor

Thanks @klijakub! I'm in and was able to push. Now - I don't think that I will be able approve this - so we may need someone else to come and help us. @josh-padnick @oredavids ?

Copy link
Contributor

@eak12913 eak12913 left a comment

Choose a reason for hiding this comment

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

I checked all the pages and saw no weird prism CSS issues (checked Guides, LZ page and the pipelines page)

@josh-padnick
Copy link
Contributor

Ugh, I'm a contributor now. @oredavids can you approve?

Copy link
Contributor

@oredavids oredavids left a comment

Choose a reason for hiding this comment

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

Rubber stamp.

@eak12913 eak12913 merged commit 4ac3142 into gruntwork-io:master Nov 20, 2020
@eak12913 eak12913 deleted the pipelines branch November 20, 2020 21:46
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.

6 participants