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

add action to deploy assets from PR and delete them when the PR is closed #159

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

deniak
Copy link
Member

@deniak deniak commented Oct 17, 2024

That PR uploads assets from pull requests to cdn-dev.w3.org and deletes them when the PR is closed

@deniak deniak changed the title add action to deploy assets from PR and delete them when the PR is cl… add action to deploy assets from PR and delete them when the PR is closed Oct 17, 2024
Comment on lines +1 to +3
on:
pull_request_target:
types: [opened, edited, reopened, synchronize]
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if deploying automatically is a good idea. It could be unsafe to deploy PRs opened by external contributors, so deploying manually might be better.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, we could use workflow_dispatch instead to be on the safe side but we will need to run it manually each time the PR is updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Running manually seems fine to me.

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 think we might be able to trigger the action only on local PRs (so from branches on that repository) instead of PRs coming from forked repositories. How does that sound?

runs-on: ubuntu-latest
env:
source: public/dist/assets
destination: assets/website-2021/pr-${{ github.event.number }}/
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we create a folder for all of them, rather than putting them directly under website-2021?

Copy link
Member Author

Choose a reason for hiding this comment

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

we could use assets/website-2021-dev/ to avoid mixing prod and dev assets.

AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
AWS_REGION: 'us-east-1'
run:
aws s3 sync --no-progress ${{ env.source }} s3://cdn-dev.w3.org/${{ env.destination }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if we want to use cdn-dev or cdn. I think cdn-dev is restricted, and if so it may be difficult to show changes to external contributors.

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.

2 participants