-
Notifications
You must be signed in to change notification settings - Fork 36
Add 'last updated' banner to internal and externally version controlled docs (NEW) #2836
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
Conversation
Unlike the 'internal' docs that live in govuk-developer-docs, these externally pulled in docs have no 'frontmatter' (YAML) describing their state. All we have to go on is the commit history itself. This commit creates a 'latest_commit' property that is proxied through to the frontend via ProxyPages. We use this to show the 'last updated' value and link to the SHA of the commit so that the viewer can see the latest changes.
Unlike the external docs which are pulled in via [middleman proxy], the internal docs are rendered 'natively' by middleman and don't expose any hooks for adding to its frontmatter. Short of hard coding each SHA and timestamp at the top of each doc's YAML file, I've had to write a helper. The helper examines the `current_page` and runs local git commands against the source file to establish the SHA and timestamp of the latest commit for the file. [middleman proxy]: https://middlemanapp.com/advanced/dynamic-pages/
We want the banner to look the same regardless of where the doc is hosted. Therefore I've moved the HTML into a shared partial that is called by the respective layouts. One caveat is that the external docs are 'unnecessarily' being passed through the CommitHelpers, but it seems reasonable to allow for this with a conditional. But the advantage of this is that we no longer need the conditional 'guard' for /apis.html. The commit-retrieval mechanism is more robust - in this case, it now fetches the commit SHA for the local /apis.html.md file.
I've opted for the same format as GitHub uses for older dates. We could in future make this more clever (like GitHub does) by omitting the year if it is the current year, and/or by making recent updates relative (e.g. "3 days ago").
Move it above the h1, as it looks better. Apply some margins and make the text smaller.
We decided to make this change because it makes the code more readable, and: - It's a small dependency, so shouldn't impact day-to-day development - It's less coupled to command line options and other commands (cut). - It's a bit of future-proofing against doing any more git commands. Co-authored-by: Ben Thorner <Ben.Thorner@digital.cabinet-office.gov.uk>
We ran into an [issue][] whereby the 'last updated' logic worked locally but not on production. All 'local' pages were showing today's date rather than the date they were last updated. I hypothesised that `current_page` evaluated to an empty string, giving the `source_file` value of `source/` for all local docs, and thereby grabbing the latest commit for any file in that folder. However, I now believe the issue lies with the way the repo was cloned using GitHub Actions. By [default][], only a single commit is fetched when cloning a repo using the 'actions/checkout' action. This is for efficiency reasons. But it means that we can't reliably query the commit history for particular files, within the action itself - everything is wrongly attributed to the single latest commit. By adding 'fetch-depth: 0', we set an infinite fetch depth. In other words, the entire repository history is cloned, so we should get accurate results when querying the commit SHA and timestamp of particular files. [default]: https://github.com/actions/checkout [issue]: #2784 (comment)
|
I've pushed up a branch to test this, by logging out the timestamps vs paths: https://github.com/alphagov/govuk-developer-docs/runs/1320989727 Sample: This appears to be working 🎉 |
| steps: | ||
| - uses: actions/checkout@v2 | ||
| with: | ||
| fetch-depth: 0 |
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.
Do we need it when running tests? Similarly for the token, now I come to think of it.
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.
We do need the token, as we run a full build as part of the test suite (to catch things like unparseable markdown, which should error the build).
We don't strictly need the fetch-depth: 0 for the tests but I'm wary of the test and build environments diverging (and causing exactly the kind of bug this commit is trying to fix!).
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.
Ah, true. I think it's OK to digress here: we do benefit from speeding up the build, and it's highly unlikely that we'll be writing a test that depends on previous commits (since a future commit may always supersede them!).
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.
Agreed the risk is low 👍 I may raise a follow-up PR at some point, concentrating on bringing the build time down more generally.
benthorner
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.
Nice one (again) tracking down the bug 💯.
(I'm assuming nothing else has changed.)
GOV.UK no longer uses 'Daniel the Manual Spaniel', since alphagov/govuk-developer-docs#2836 and alphagov/govuk-developer-docs#2837 were merged.
* Bump addressable from 2.5.2 to 2.8.0 Bumps [addressable](https://github.com/sporkmonger/addressable) from 2.5.2 to 2.8.0. - [Release notes](https://github.com/sporkmonger/addressable/releases) - [Changelog](https://github.com/sporkmonger/addressable/blob/main/CHANGELOG.md) - [Commits](sporkmonger/addressable@addressable-2.5.2...addressable-2.8.0) --- updated-dependencies: - dependency-name: addressable dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * Remove GOV.UK Developer Docs from the notify list GOV.UK no longer uses 'Daniel the Manual Spaniel', since alphagov/govuk-developer-docs#2836 and alphagov/govuk-developer-docs#2837 were merged. * Bump rake from 13.0.3 to 13.0.6 Bumps [rake](https://github.com/ruby/rake) from 13.0.3 to 13.0.6. - [Release notes](https://github.com/ruby/rake/releases) - [Changelog](https://github.com/ruby/rake/blob/master/History.rdoc) - [Commits](ruby/rake@v13.0.3...v13.0.6) --- updated-dependencies: - dependency-name: rake dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * TRG-139: Support sites which use relative URLs. This involves using the site's URL, in combination with the relative URLs returned in `/api/pages.json` and using them to build absolute an absolute URL to each page which is to be notified upon. Sites which use absolute links are unaffected. * Add gitignore This project currently lets you commit the `.bundle` folder, if you're not careful. Add a copy of the GitHub suggested gitignore file for Ruby projects [[1]], to make it harder to do that. [1]: https://github.com/github/gitignore/blob/ce5da10a3a43c4dd8bd9572eda17c0a37ee0eac1/Ruby.gitignore * Bump http from 3.3.0 to 5.0.4 Bumps [http](https://github.com/httprb/http) from 3.3.0 to 5.0.4. - [Changelog](https://github.com/httprb/http/blob/main/CHANGES.md) - [Commits](httprb/http@v3.3.0...v5.0.4) Breaking changes don't affect this project. This should also fix any issues from frozen string weirdness in later versions of Ruby (see [[1]], [[2]]). [1]: alphagov#40 (comment) [2]: httprb/http#653 * Bump Ruby to latest 2.x release Ruby 2.6 is in security maintenance phase and will reach end-of-life soon [[1]]. This commit changes the `.ruby-version` file used by developers and tooling to use the current latest release of 2.7, which is the last in the 2.x series. [1]: https://www.ruby-lang.org/en/downloads/branches/ * Fix Concourse pipeline Missed this from commit d375525. * Bump bundler from 1.7.5 to 2.1.4 Bumps bundler to the same version as used in the current ruby:2.75 Docker image (280416eb1a1c). * Reorder and group dependencies * Bump vcr from 4.0.0 to 6.0.0 Bumps [vcr](https://github.com/vcr/vcr) from 4.0.0 to 6.0.0. - [Release notes](https://github.com/vcr/vcr/releases/tag/v6.0.0) - [Changelog](https://github.com/vcr/vcr/blob/master/CHANGELOG.md) - [Commits](vcr/vcr@v4.0.0...v6.0.0) Breaking changes don't affect this project. * Remove default help text from spec_helper file * Move VCR configuration to spec_helper file * Add VCR RSpec metadata configuration Needed to make `vcr: true` work. * Use VCR RSpec metadata instead of before and after blocks * Add tests for Notifier#run * Add support for Slack chat.postMessage API If the environment variable `SLACK_TOKEN` is set use Slack's REST API [1] instead of webhooks. This is needed because new-style webhooks generated by Slack apps cannot post to any channel [2]; instead we need to use an OAuth token with scopes chat:write and chat:write.public. [1]: https://api.slack.com/methods/chat.postMessage [2]: https://api.slack.com/messaging/webhooks * Replace Concourse pipeline to run monitor with GitHub Actions workflow We want to use GitHub Actions everywhere, instead of having some of the infra running on a Concourse installation. This commit replaces the Concourse CI pipeline with a GitHub Actions workflow. Note that although the schedules are configured to be similar, they won't be exactly the same, as concourses' time resource behaves differently to cron. Also, the GitHub Actions scheduled event doesn't allow configuration of timezone, so the midday-ness of the new scheduled messages will be subject to daylight savings. Another change from the Concourse pipeline is that we now use a Slack token rather than a webhook. This is so that we can use a Slack app to configure the secrets for the integration, rather than the deprecated webhooks. This should hopefully make sharing the control of the integration among multiple people easier. * Remove unneeded notify file Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Chris Ashton <ChrisBAshton@users.noreply.github.com> Co-authored-by: Laurence de Bruxelles <laurence.debruxelles@digital.cabinet-office.gov.uk> Co-authored-by: edd.grant <edd@eddgrant.com> Co-authored-by: Philip Potter <philip.g.potter@gmail.com>
This PR is identical to #2784 other than one final commit which fixes a deployment issue we were seeing, and caused us to have to revert.
This PR removes the old "last reviewed on" banner from the bottom of the internally hosted docs. In their place - and as a new feature for the externally hosted docs - there's now a "Last updated" banner at the top of each doc. The date is clickable and links to the commit SHA on GitHub so that the changes can be viewed easily.
"Last updated" on internal docs:
"Last updated" on external docs:
This is phase 1 of removing Daniel the Manual Spaniel. Once the 'last reviewed' date is no longer surfaced visually, there's no point in keeping it - we'll remove the 'last_reviewed' and 'review_in' properties from files and discontinue the spaniel.
Trello: https://trello.com/c/OMBV5qM4/198-retire-daniel-the-manual-spaniel