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

fix: WIP - Addressing feedback from #1683 #1699

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yhakbar
Copy link
Contributor

@yhakbar yhakbar commented May 8, 2024

Addressing comments in #1683

Copy link

netlify bot commented May 8, 2024

Deploy Preview for pensive-meitner-faaeee ready!

Name Link
🔨 Latest commit 757776e
🔍 Latest deploy log https://app.netlify.com/sites/pensive-meitner-faaeee/deploys/663cd26d45282d0008bb855b
😎 Deploy Preview https://deploy-preview-1699--pensive-meitner-faaeee.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@josh-padnick
Copy link
Contributor

I decided to review the work in progress and had a few thoughts in no particular order:

  • Where do the architecture components live? On https://deploy-preview-1699--pensive-meitner-faaeee.netlify.app/pipelines/architecture/, we describe the architecture components, but we don't say where they live, how they're invoked, etc. Perhaps we could make this concrete. For example, "the orchestrator is a Go binary provided by Gruntwork that is invoked by your GitHub Actions workflow after you make a commit or merge."
  • A picture's worth 1,000 words. A simple, rough visual diagram showing a basic information flow could go a long way to helping users understand how Pipelines works.
  • Instructions on how to use Pipelines. I think I mentioned this in my last feedback, but whether as part of this PR or a follow-on one, we could use instructions on how to use Pipelines. For example, you make a commit, and that automatically invokes Pipelines. Ideally, we could include screenshots, though those will all change in a few days once we finish the UX polish.
  • Explicit installation instructions. We should also have an explicit page on how to install Pipelines, even if it requires DF to do so now and the user can't actually install it themselves.
  • We should work in small batches. Large PRs are tough to work on, so perhaps do what you can to get this PR over the finish line, and we'll figure out updates to these other items in upcoming PRs.

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.

None yet

2 participants