Rewrite README to be consistent with other repos #2108
Merged
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
added 5 commits
May 17, 2021 09:19
This takes up too much room for the little value it adds, being only specific to one of the many, many document type this app caters for, which can better be viewed live, or by starting the app.
This content is way too long for a README, and isn't necessary to get started with the app.
Previously this was split between the README and an existing doc, with no link between the two.
This is too vague to be actionable, and Content Store / Static are common dependencies for frontend apps that we can document centrally. I've kept the reference to ImageMagick, as it links in with existing docs, but it's unclear why anyone needs to know about PhantomJS in order to work on this repo.
Arguably this is redundant, as GOV.UK Publishing Components is used this way in many other repos. As a compromise, I've simplified the content of the section, retaining all the links.
This follows the approach we've taken in other apps [1]. I've kept the reference to Content Store in the README intro, and removed the vague part about how `save_and_open_page` is "helpful". [1]: alphagov/collections#2375
benthorner
force-pushed
the
rewrite-readme
branch
from
0eaba92 to
62b3677
Compare
kevindew
reviewed
May 18, 2021
| @@ -4,10 +4,6 @@ Government Frontend is a public-facing app to display the majority of documents | |||
| on the /government part of GOV.UK. It is a replacement for the public-facing | |||
| parts of the [Whitehall](https://github.com/alphagov/whitehall) application. | |||
|
|
|||
| ## Screenshots | |||
|
|
|||
|  | |||
There was a problem hiding this comment.
Makes sense - given the screenshot is so text heavy it's quite confusing.
|
|
||
| - [content-store](https://github.com/alphagov/content-store) - provides documents | ||
| - [static](https://github.com/alphagov/static) - provides shared GOV.UK assets and templates. | ||
| - [phantomjs](http://phantomjs.org/) Used by poltergeist for integration testing |
There was a problem hiding this comment.
I imagine this phantomJS reference was out of date anyway - at least I hope so 🤞
README.md
Outdated
| If you want to see the page that is being tested in our integration tests, you can use | ||
| `save_and_open_page` to see what's rendered. This is helpful when a page is mostly comprised of | ||
| GOV.UK Publishing Components | ||
| If you want to see the page that is being tested in our integration tests, you can call `save_and_open_page` to see what's rendered. |
There was a problem hiding this comment.
Do we know if this tip still works when running the tests in govuk-docker?
It seems a little inconsistent to have this anyway since it's just a capybara feature and this tip seems to miss most of the info you'd need to actually utilise it.
There was a problem hiding this comment.
True, it doesn't. I've removed this in 7c1903e.
README.md
Outdated
|
|
||
| Components specific to government-frontend are [within the application](https://github.com/alphagov/government-frontend/tree/master/app/views/components) and follow rules set out by the [govuk_publishing_components](https://github.com/alphagov/govuk_publishing_components) gem. They are documented in the [government-frontend component guide](https://government-frontend.herokuapp.com/component-guide). | ||
|
|
||
| Configuration for the navigation links shown on the B variant of the ContentPagesNav A/B test is covered [separately](docs/navigation-links.md) |
There was a problem hiding this comment.
This links to a 404 and seems unrelated to components - I'd suggest removing.
There was a problem hiding this comment.
Good spot, I've removed it in a79d31d.
added 2 commits
May 19, 2021 17:37
This doesn't work with GOV.UK Docker since the screenshots are saved to the "tmp" directory, which is a temporary volume. We could change the guidance to advise specifying a path. On the other hand, a README isn't the right place to be documenting features of a third-party library, which aren't specific to this repo or GOV.UK. We can always write a dev doc if necessary.
This was removed ages ago [1], with no clear replacement. [1]: #1468
|
Thanks for the deeper checking @kevindew 💯. I think this one is ready for another look now. |
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.
https://trello.com/c/Psf7EzPB/204-move-away-from-startupsh
Please see the commit messages for more details. I've kept the
long "schemas" section, since it's a comprehensive description
of what the repo does, with actionable and illustrative info for
each of the (non-)migrated docs it renders.