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

Feat: PR Preview Workflow via GitHub Pages #4848

Merged
merged 3 commits into from
Aug 5, 2024

Conversation

AdarshRawat1
Copy link
Member

@AdarshRawat1 AdarshRawat1 commented Aug 2, 2024

This PR improves the existing functionality of deploying the main branch to GitHub Pages, providing an additional layer of testing for Documentation PRs by enabling previewing pull requests via GitHub Pages.

Changes

  • PR Preview: Each pull request with documentation tag will trigger a preview build that is deployed to a unique GitHub Pages URL. The preview is dynamically updated with the latest changes from the PR.
  • Comment Updates: automatic comments on PRs with deployment status and links to the preview environment.
  • Handling Broken Builds: The workflow includes functionality to detect and comment on broken builds,.

Benefits

This update enhances Documentation PR review process & provides faster Iterations by offering a real-time preview of documentation changes on GitHub Pages. The preview automatically updates with each PR change, enabling immediate feedback and faster iterations. It also provides automatic notifications for build failures.

Usage

Once a PR tagged with documentation label, the workflow will trigger and deploy the preview to GitHub Pages.
A comment will be posted on the PR with the link to the preview and deployment status (Passed/Failed).

Test and Review this workflow

Please review and test this new feature on the test documentation repo and check the following sample PR for reference .

Additional Context

This update addresses a concern raised in a PR review comment about testing Docs PRs locally before merging. For more details on how to test locally, see my comment.

While local testing is possible, it can be redundant for maintainers. Tracking PR changes and syncing with remote updates can be difficult, leading to version mismatches and inconsistencies.

@fruffy fruffy added infrastructure Topics related to code style and build and test infrastructure. documentation Topics related to compiler documentation. labels Aug 2, 2024
Signed-off-by: Adarsh <Adarshbunny293@gmail.com>
Signed-off-by: Adarsh <Adarshbunny293@gmail.com>
Signed-off-by: Adarsh <Adarshbunny293@gmail.com>
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}

- name: Check PR Label
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I initially attempted to use the pull_request trigger, but it’s restricted by security rules for commenting on PR from forks.

We need to use pull_request_target for commenting because it has the required access to the base repository. However, it doesn’t provide an easy way to fetch labels directly. To handle this, I used actions/github-script to retrieve labels through GitHub's REST API, which works for forked PRs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm I see, this workflow is very complex it could be hard to maintain.

Let's merge it and see how it behaves.

Copy link
Collaborator

@fruffy fruffy left a comment

Choose a reason for hiding this comment

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

Great work! The best way to test it is to merge and see how the PR workflow behaves going forward.

@fruffy fruffy enabled auto-merge August 5, 2024 19:37
@fruffy fruffy added this pull request to the merge queue Aug 5, 2024
Merged via the queue into p4lang:main with commit ab33c0a Aug 5, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Topics related to compiler documentation. infrastructure Topics related to code style and build and test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants