Skip to content

ci: setup automatic deployment for the docs app #24528

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

Merged
merged 2 commits into from
Mar 10, 2022

Conversation

devversion
Copy link
Member

@devversion devversion commented Mar 5, 2022

See the following document for the overall plan that this
commit implements:

https://docs.google.com/document/d/1xkrSOFa6WeFqyg1cTwMhl_wB8ygbVwdSxr3K2-cps14/edit

We will use a test firebase project temporarily to ensure its working as intended.

@devversion
Copy link
Member Author

devversion commented Mar 5, 2022

Note: As mentioned in the "proposal doc", moving the docs repo into angular/components has various benefits. Long-term we should do this, or as a follow-up. The PR is conceptually ready for review, just contains some testing TODOs still.

@devversion devversion force-pushed the docs-app-deployment branch from 11dc3bf to 4c77c55 Compare March 5, 2022 16:04
@devversion devversion added merge safe target: patch This PR is targeted for the next patch release labels Mar 5, 2022
@devversion devversion requested a review from gkalpak March 5, 2022 16:13
@devversion
Copy link
Member Author

devversion commented Mar 5, 2022

I chatted about this to a good extent with @gkalpak. Long-term we certainly want to share the same script from AIO, and potentially also use redirects (e.g. where rc.angular.io would just redirect to angular.io if there is no RC -- but that is out of scope for now). The current changes include:

  • A TS-converted, reduced conceptual replica of the angular/angular script -- but leveraging @angular/dev-infra-private instead of manually consulting Git for release train info.
  • Logic for cloning the docs-repo (no caching for now; seems fast enough), and running monitoring tests as a cronjob to ensure material.angular.io remains working (we don't have any good integration tests..)
  • Preparation logic for having docs-app snapshots for every commit (using preview channel deployments)

@devversion devversion marked this pull request as ready for review March 5, 2022 19:11
@devversion devversion requested a review from a team as a code owner March 5, 2022 19:11
@devversion devversion requested a review from crisbeto March 5, 2022 19:15
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@devversion devversion force-pushed the docs-app-deployment branch 2 times, most recently from 36ebd37 to 182c8ec Compare March 8, 2022 20:20
@devversion
Copy link
Member Author

@josephperrott addressed your feedback and also tested on my fork again. We'll also need a patch-port

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Overall lgtm (with a few comments) expect for the test values (e.g. for branch names, Firebase project/site IDs, remote URLs, etc.) that need to be updated.

👍

Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

@devversion devversion force-pushed the docs-app-deployment branch from 182c8ec to d6c193e Compare March 9, 2022 13:08
@devversion devversion requested a review from gkalpak March 9, 2022 13:12
@devversion devversion force-pushed the docs-app-deployment branch 3 times, most recently from 98767b0 to 1bddbc7 Compare March 9, 2022 18:36
Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@devversion devversion force-pushed the docs-app-deployment branch from 1bddbc7 to 0f2fd13 Compare March 9, 2022 19:50
@devversion devversion added the action: merge The PR is ready for merge by the caretaker label Mar 9, 2022
@devversion
Copy link
Member Author

Note: Corresponding patch PR that needs to go in once this gets merged: #24552

@devversion devversion added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Mar 9, 2022
@devversion devversion force-pushed the docs-app-deployment branch from 13c1a84 to 71b586e Compare March 9, 2022 21:07
@devversion devversion added merge: preserve commits When the PR is merged, a rebase and merge should be performed target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Mar 9, 2022
We currently configure RBE by setting `GOOGLE_APPLICATION_CREDENTIALS`
into the `$BASH_ENV` variable, ensuring RBE is configured everywhere on CI.

This worked nicely but now with automatic docs deployment turned out to
be problematic since it prevents scripts from defining `GOOGLE_APPLICATION_CREDENTIALS`
themselves/overriding it. the reason is that `$BASH_ENV` always runs in new
child processes (like when firebase is initialized) and then overrides the credentials
back to the RBE service key.

We can simplify this code by using a dedicated Bazel flag.
@devversion devversion force-pushed the docs-app-deployment branch from 71b586e to a59977e Compare March 10, 2022 15:28
@zarend zarend merged commit 26fc03e into angular:master Mar 10, 2022
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note merge: preserve commits When the PR is merged, a rebase and merge should be performed target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants