-
Notifications
You must be signed in to change notification settings - Fork 1k
Conversation
Gives a better name for CI results on GitHub
|
BTW I manually tested what this pr will automate and teh result is live at https://staging.nodejs.dev |
| args: ['run', 'build'] | ||
| - name: 'gcr.io/cloud-builders/gsutil' | ||
| args: ['-m', 'rsync', '-R', 'public', 'gs://staging.nodejs.dev/$SHORT_SHA'] | ||
| args: ['-m', 'rsync', '-R', 'public', 'gs://staging.nodejs.dev/'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is deploying to the root of staging, is that going to conflict/overwrite those directories?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't be. the directories for staging are numbers generated from PR number. AFAIK there is no directory generated by the website that is just an integer.
If we wanted to be safe we could move the generation of preview to either another subdomain or a folder down such as staging.nodejs.dev/preview
We would just have to make sure to not reuse that folder for the website.
benhalverson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @MylesBorins 👍
|
I accidentally hi squash and merge oops. I force pushed over the staging branch with the original commits maintained (will make future rebasing easier). Things are now landed in 160a67c...b290d65 Was only up for less than 5 seconds so hopefully didn't break anyones workflow |
This PR gets the CI / CD + Actions on staging up to date with master. This will get /preview working as well as allow us to get builds up on staging.nodejs.dev for the latest version of the staging branch.
I cherry-picked all the important commits and added an additional commit that will publish the result of this branch to staging.nodejs.org.
One thing I noticed is that we have been doing a merge strategy up until now, which makes it easier to dev against the branch but geniunely makes it a bit more difficult to reason about the difference between staging and master. if folks are up for it I'd like to follow this up with a rebase of staging against master so it is above the head and explore having CI to automate the process of keeping it that way. It will make development a bit more difficult but it will definitely give folks a heads up about conflicts super early and make sure an eventual merge won't be hard.
It looks like #654 might already do a good chunk of the rebase. @benhalverson if your branch is essentially a rebase it might make sense to just replace staging with it after we land this.