Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update govuk_tech_docs to the latest version #632

Merged
merged 2 commits into from
Jul 30, 2021
Merged

Update govuk_tech_docs to the latest version #632

merged 2 commits into from
Jul 30, 2021

Conversation

timblair
Copy link
Member

@timblair timblair commented Jul 28, 2021 •
edited
Loading

This includes a number of accessibility fixes, but one a11y issue remains, so the accessibility statement has been updated to reflect this.

@36degrees 36degrees mentioned this pull request Jul 28, 2021
Closed
@timblair timblair marked this pull request as ready for review July 29, 2021 14:27
@timblair
Update accessibility statement
1447b70 
The accessibility statement is currently out of date. As of v2.4.0 of the
govuk_tech_docs gem (alphagov/tech-docs-gem#247)
there is one outstanding accessibility which prevents the site being fully
compliant with WCAG AA.
@tombye
Copy link
Contributor

tombye commented Jul 29, 2021

I don't really do much ruby so might be wrong here but shouldn't the first commit include the Gemfile too?

@timblair
Copy link
Member Author

Nope, the Gemfile just says gem 'govuk_tech_docs'. It's the Gemfile.lock that sets the version.

tombye
tombye requested changes Jul 29, 2021
View reviewed changes
Copy link
Contributor

@tombye tombye left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The changes to the statement look good. I assume this will just pull in version 2.4.2 of govuk_tech_docs? it didn't for me locally running bundle install but testing with that version looked good.

The only change I'd like is for the Gemfile to lock ffi to 1.12, (like alphagov/datagovuk-tech-docs#60), to stop it blowing up for users on OSX :)

@timblair
Copy link
Member Author

I assume this will just pull in version 2.4.2 of govuk_tech_docs? it didn't for me locally running bundle install but testing with that version looked good.

Are you sure you were on the correct branch? I just did a fresh clone and bundle install on the bump-tech-docs branch and it installed 2.4.2 as expected.

The only change I'd like is for the Gemfile to lock ffi to 1.12, (like alphagov/datagovuk-tech-docs#60), to stop it blowing up for users on OSX :)

Is this a common thing? The GDS Way repo has been using a version greater than 1.12 for a fair while. I've also never seen the problem listed in that issue (using MacOS). ffi is being pulled in as a dependency from govuk_tech_docs. If it needs pinning, it should be done there?

@tombye
Copy link
Contributor

tombye commented Jul 29, 2021

Are you sure you were on the correct branch? I just did a fresh clone and bundle install on the bump-tech-docs branch and it installed 2.4.2 as expected.

I am sure about the branch. I just tried again and got 2.4.2 so maybe a timing thing? Either way, I'm happy this is not a problem.

Is this a common thing? The GDS Way repo has been using a version greater than 1.12 for a fair while. I've also never seen the problem listed in that issue (using MacOS).

There are a few GDS'ers on that issue reporting experiencing it and I know of a few others doing the same on Slack a while ago. From reading the issue, it looks like it might effect people on Mohave only:

An upgrade to MacOS10.15 should help

ffi is being pulled in as a dependency from govuk_tech_docs. If it needs pinning, it should be done there?

Makes sense. I'll open an issue on that repo instead.

tombye
tombye approved these changes Jul 29, 2021
View reviewed changes
Copy link
Contributor

@tombye tombye left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍🏻

@tombye tombye mentioned this pull request Jul 29, 2021
Closed
@timblair timblair merged commit 8abdbc5 into main Jul 30, 2021
@timblair timblair deleted the bump-tech-docs branch July 30, 2021 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tombye tombye tombye approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@timblair @tombye