Skip to content

Steal first version of tech docs from teacher training api#1172

Merged
kicferk1 merged 16 commits intodevelopfrom
api-docs-fun
Oct 5, 2021
Merged

Steal first version of tech docs from teacher training api#1172
kicferk1 merged 16 commits intodevelopfrom
api-docs-fun

Conversation

@kicferk1
Copy link
Copy Markdown
Contributor

@kicferk1 kicferk1 commented Oct 1, 2021

Ticket and context

Ticket: https://dfedigital.atlassian.net/browse/CPDTP-477

I had a little dig, and it seemed to me that what the docs looked in the ticket was eerily similar to what TeacherTraining API were doing. Which is using a gem to generate their documentation.

Vast majority of changes are caused by a different way of automatically presenting API operations - I cut out everything we stole from Apply for Teacher Training, and brought in everything from TeacherTraining API.

The proof of the pudding is in the eating, so dig in - https://ecf-review-pr-1172.london.cloudapps.digital/api-reference

The technical gist of how the docs are handled: We use govuk_tech_docs to harness power of middleman to generate a static site from the stuff we put in docs directory. We output that static site into public/api-reference at build time - so when we run our application, it is served from /api-reference path. One drawback of it which I haven't found a solution to is that running percy and / or axe on it seems harder. Since it would require the static site to be built before we ran the feature specs :/ I added some basic smoke tests but maybe they are not quite enough. Open to suggestions.

Getting that api-reference path to work was harder than expected, cause the gem is a bit opinionated - I needed to override some methods generating headings on left hand side, because without it they thought everything should be in one big family under api-reference.

Open to suggestions of what I missed :)

@kicferk1 kicferk1 added the blocked-merge Do not merge this PR label Oct 1, 2021
@kicferk1 kicferk1 marked this pull request as draft October 1, 2021 11:08
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 1, 2021

Created review app at https://ecf-review-pr-1172.london.cloudapps.digital

@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 1, 2021

Smoke tests passed against the review app.

@kicferk1 kicferk1 removed the blocked-merge Do not merge this PR label Oct 4, 2021
@kicferk1 kicferk1 marked this pull request as ready for review October 4, 2021 14:48
@kicferk1 kicferk1 requested a review from a team October 4, 2021 14:58
Comment thread config/routes.rb

get "/how-to-set-up-your-programme", to: "step_by_step#show", as: :step_by_step

get "/assets/govuk/assets/fonts/:name.:extension", to: redirect("/api-reference/assets/govuk/assets/fonts/%{name}.%{extension}")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not sure we want this? every asset will require an extra request assuming it's not cached. I feel like this is a workaround and doesn't solve the actual problem we have introduced.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The problem is that the gem govuk_tech_docs hard-codes its assets to be in /assets/govuk/assets/, and I am not sure it's a good place for something that's a small part of our application.

govuk_tech_docs in general doesn't seem to believe it should be just a small part of an application. But I think it is a pretty good idea to have the tech docs be part of what we release, and this seemed like the simplest solution. I guess we could make it a separate mini-service in paas, but that would require a lot of dev ops fun :/

Copy link
Copy Markdown
Contributor Author

@kicferk1 kicferk1 Oct 4, 2021

Choose a reason for hiding this comment

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

We could get rid of it when alphagov/tech-docs-gem#197 goes in, or something similar to it.

Comment thread docs/.tool-versions Outdated
@kicferk1 kicferk1 enabled auto-merge October 5, 2021 12:48
@kicferk1 kicferk1 merged commit ff8fbbd into develop Oct 5, 2021
@kicferk1 kicferk1 deleted the api-docs-fun branch October 5, 2021 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants