WIP: Refactor build task, caching mechanisms and middleman frontmatter #2916
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The method does actually return an array of elements of type `App`, so `pages` is quite a disingenuous name, all the more problematic now that we have a concept of imported app 'docs' (synonymous with pages). Renaming 'pages' to 'apps' makes it clearer what object types we are dealing with.
ChrisBAshton
force-pushed
the
refactor-app-docs
branch
4 times, most recently
from
9925d56
to
a4ddba8
Compare
We're already checking if a repo is private in the 'consume_docs_folder' method in AppDocs, but this will be removed in the next commit. It's safer to move the check to a lower level in the code, to ensure that there's no possible way we can accidentally retrieve private information through these methods. I had to stop using `app_name` and start using `github_repo_name` for `govuk-content-store-examples`, which is actually called `govuk-content-schemas` on GitHub. I also had to explicitly reject private repos from ProxyPages and api_layout, as otherwise the build would error with `RuntimeError: alphagov/govuk-knowledge-graph not found` (which is a private repository). Hopefully we can continue to refactor govuk-developer-docs to be less brittle around this.
ChrisBAshton
force-pushed
the
refactor-app-docs
branch
from
a4ddba8
to
ffb8776
Compare
Since #2910, _all_ apps have their `docs/` folder imported. We have not identified a reason why any of them should be opted out, so to simplify the code & tests, we've now removed the relevant code.
- Move govuk-content-schemas check later on in the task, as it's quite slow. - Suppress the very noisy `HTTP.get` output from the `verify_deployable_apps` and `check_puppet_names` tasks, so it's easier to figure out where in the task we are. - Remove the `/tmp/govuk-content-schemas` before cloning, to a) stop the task from borking when this dir already exists, and b) ensure it is always up to date. - Clarify that you do now NEED a GitHub auth token to build or test govuk-developer-docs, given that there are so many API requests made now that we import external documentation. Documents a useful shortcut for retrieving your auth token without exposing it in your bash history.
This prevents us from clearing the cache, which _hugely_ speeds up being able to run a full test suite as we no longer have to fetch all the docs from GitHub again. It is safe to do so here because the actual deployment calls `build` directly (https://github.com/alphagov/govuk-developer-docs/blob/55ffdc5e34cd1a3168f31f7fa3e29a5e803fec3b/.github/workflows/ci.yml#L52) which does a cache clear before the build. So deployments are unaffected. This change only makes life easier locally. HOWEVER - this is a WIP because many tests rely on clearing the cache and putting arbitrary things in it. So we need to rewrite those tests to mock the cache instead. I've had to expose a `cache=` method to allow tests to write a different cache object. Ideally we shouldn't edit application code for the sake of our tests, but as this isn't an API people are using then I don't see much of an issue. I've removed the `@all_alphagov_repos ||=` cache, which was making it difficult to write tests. It also wasn't very effective as an optimisation tool, as it was only saving a (very cheap) lookup in the cache, rather than say network requests.
ChrisBAshton
force-pushed
the
refactor-app-docs
branch
from
ffb8776
to
9e48b5a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commits for details.
TODO: make tests less horrific.