Support --target_branch #877
Conversation
Adds support for specifying the branch to which to build docs in the target repo. This will make release times smoother because we can continue to build the docs but send them to the `staging` branch. It *might* allow more fancy pull request testing by allowing us to send pull requests to their own branch. If the branch already exists we just check it out and build the docs onto it and then push it. If it doesn't exist then things are a little more complicated. First we check if the `master` branch exists. It won't in an entirely fresh repo which we often use for testing. If it doesn't exist we create an empty commit in the master branch to "anchor" it. Once we're sure that the master branch exists we fork the new target branch from the master branch. Pushing the built docs gets a bit more complex in two ways: 1. If the target branch is new we always push it, even if there haven't been any changes. This keeps things consistent, both for easy testing and for the potential PR testing work that I mentioned above. 2. If we had to create an empty master branch we push that too. This makes sure that non-empty repos have a master branch which is a fairly useful invariant.
| @@ -29,7 +29,6 @@ BEGIN | |||
| run $Opts | |||
| build_chunked build_single build_pdf | |||
| proc_man | |||
| sha_for | |||
| @@ -29,7 +29,6 @@ BEGIN | |||
| run $Opts | |||
There was a problem hiding this comment.
The perl is at the top of this diff and at the bottom....
| } | ||
|
|
||
| my $remote_sha = eval { |
There was a problem hiding this comment.
I'm fairly sure we don't need to check the upstream's sha like this. This seems like it is for catching local commits and pushing them upstream while you build the docs. That made more sense when the built docs were in the same repo as the docs code but at this point I don't think it is a thing we need. It should be safe enough to reason about whether or not we should push based on if we made changes that need to be pushed.
| @@ -4,42 +4,50 @@ | |||
| # Assertions about when books are rebuilt based on changes in source | |||
| # repositories or the book's configuration. | |||
| RSpec.describe 'building all books' do | |||
| class Config | |||
There was a problem hiding this comment.
I didn't want to add this as a variable to Dest because it is really test config. So I made this....
| @@ -50,13 +58,15 @@ def self.build_one_book_out_of_two_repos_twice( | |||
| before_second_build: ->(src) {} | |||
| ) | |||
| build_twice( | |||
There was a problem hiding this comment.
I figure I don't need to support conf on the "build one book out of two repos twice" case yet. When I do I'll add it. But I don't for now to prevent noise in the PR.
| build_one_book_out_of_one_repo_twice( | ||
| before_second_build: lambda do |src| | ||
| before_second_build: lambda do |src, _config| |
There was a problem hiding this comment.
I love that rubocop forces me to to _ on the front of a variable I'm not using. It is super nice to know that this variable is entirely there to satisfy some interface.
| end | ||
| context "when the second build isn't a noop" do | ||
| let(:latest_revision) { 'changed text' } |
There was a problem hiding this comment.
At this point more than half of the examples override this so it isn't worth keeping is as the default.
| @@ -7,7 +7,7 @@ use v5.10; | |||
| use Cwd; | |||
| use Path::Class(); | |||
| use Encode qw(decode_utf8); | |||
| use ES::Util qw(run sha_for); | |||
There was a problem hiding this comment.
sha_for is no longer used.
|
@olksdr could you have a look a this one? |
|
Thanks for reviewing @olksdr ! |
Adds support for specifying the branch to which to build docs in the target repo. This will make release times smoother because we can continue to build the docs but send them to the `staging` branch. It *might* allow more fancy pull request testing by allowing us to send pull requests to their own branch. If the branch already exists we just check it out and build the docs onto it and then push it. If it doesn't exist then things are a little more complicated. First we check if the `master` branch exists. It won't in an entirely fresh repo which we often use for testing. If it doesn't exist we create an empty commit in the master branch to "anchor" it. Once we're sure that the master branch exists we fork the new target branch from the master branch. Pushing the built docs gets a bit more complex in two ways: 1. If the target branch is new we always push it, even if there haven't been any changes. This keeps things consistent, both for easy testing and for the potential PR testing work that I mentioned above. 2. If we had to create an empty master branch we push that too. This makes sure that non-empty repos have a master branch which is a fairly useful invariant.
Adds support for specifying the branch to which to build docs in the
target repo. This will make release times smoother because we can
continue to build the docs but send them to the
stagingbranch. Itmight allow more fancy pull request testing by allowing us to send
pull requests to their own branch.
If the branch already exists we just check it out and build the docs
onto it and then push it. If it doesn't exist then things are a little
more complicated. First we check if the
masterbranch exists. It won'tin an entirely fresh repo which we often use for testing. If it doesn't
exist we create an empty commit in the master branch to "anchor" it.
Once we're sure that the master branch exists we fork the new target
branch from the master branch.
Pushing the built docs gets a bit more complex in two ways:
been any changes. This keeps things consistent, both for easy testing
and for the potential PR testing work that I mentioned above.
makes sure that non-empty repos have a master branch which is a fairly
useful invariant.