Skip to content

Conversation

@tannewt
Copy link
Member

@tannewt tannewt commented Nov 30, 2021

Run the Jekyll build on pull_request as well to make sure it works

Run the Jekyll build on pull_request as well to make sure it works
makermelissa
makermelissa previously approved these changes Nov 30, 2021
Copy link
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for updating the URLs.

@makermelissa
Copy link
Collaborator

FYI, looks like the build action is failing.

@jwcooper
Copy link
Member

Possibly a permission issue, as pull requests are from forks and not a trusted source. build.yml uses github secrets.

More info here on how to fix it:
https://dev.to/petrsvihlik/using-environment-protection-rules-to-secure-secrets-when-building-external-forks-with-pullrequesttarget-hci

Example Brent did yesterday:
adafruit/Adafruit_IO_Python@6587c92

@jepler
Copy link
Contributor

jepler commented Nov 30, 2021

deploy should be guarded by an if: condition, then?

@tannewt
Copy link
Member Author

tannewt commented Nov 30, 2021

I think the build step is doing fancier things than just building the site like fetching and committing new files. Could we split the data update from the build so that 1) we don't build on a schedule and 2) everyone can build on their branch.

@tannewt
Copy link
Member Author

tannewt commented Nov 30, 2021

I think the build step is doing fancier things than just building the site like fetching and committing new files. Could we split the data update from the build so that 1) we don't build on a schedule and 2) everyone can build on their branch.

I'm wrong about this. The build step was fine. I'll add a condition on the deploy.

@tannewt
Copy link
Member Author

tannewt commented Nov 30, 2021

Build is green @makermelissa. I had to prevent the deploy when running on a PR.

Copy link
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Awesome, that works for me.

@makermelissa
Copy link
Collaborator

Not sure if you're waiting on Justin to also review.

@tannewt tannewt merged commit d8911f7 into adafruit:master Nov 30, 2021
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.

4 participants