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

Content Update (21092016) for State pension through partner Smart Answer #2777

Merged
merged 3 commits into from
Oct 13, 2016
Merged

Content Update (21092016) for State pension through partner Smart Answer #2777

merged 3 commits into from
Oct 13, 2016

Conversation

ikennaokpala
Copy link
Contributor

@ikennaokpala ikennaokpala commented Oct 7, 2016 •
edited
Loading

Supersedes #2738

Trello card

Description

Content changes made to the outcomes for this (State pension through partner) smart answer to correct some errors and typos.

Factcheck

Preview link
GOV.UK

Expected changes

  • Content updates to various sections and outcomes

Before

screen shot 2016-10-07 at 16 08 07

After

screen shot 2016-10-07 at 16 08 20

Affected outcomes

@ikennaokpala ikennaokpala force-pushed the state-pension-through-partner/content-edits-21092016 branch from 2c066a1 to 2e2264e Compare October 7, 2016 14:59
@ikennaokpala ikennaokpala mentioned this pull request Oct 7, 2016
Closed
@selfthinker
Copy link
Contributor

Code-wise this LGTM.
I only wonder if we should rather squash some of the commits and rewrite the commit messages to make the commit history cleaner? If no-one objects, I can do that tomorrow.

@ikennaokpala ikennaokpala force-pushed the state-pension-through-partner/content-edits-21092016 branch 2 times, most recently from 225c221 to 21a3651 Compare October 13, 2016 10:16
@ikennaokpala
Copy link
Contributor Author

@selfthinker I have consolidated the commits as requested..

@selfthinker
Copy link
Contributor

I would squash the first 3 commits as they are all the same change. I would add the 4th commit to it as well as we would usually add 2i changes to the original change. I suspect the first 3 commits were only done separately because GitHub's interface doesn't make it easy to do them together.

I would also rewrite the commit messages as they are reiterating what the exact change is and is therefore duplicating things unnecessarily. I would keep the heading and the reason, e.g.

  1. Lower case for ‘reduced rate election’

CHANGE
The Reduced Rate Election reduced rate election must have applied to you at some point within the 35 years...

TO
The reduced rate election reduced rate election must have applied to you at some point within the 35 years…

REASON
Not GOV.UK style to capitalise this.

would change to

  1. Lower case for ‘reduced rate election’ because it's not GOV.UK style to capitalise this.

@ikennaokpala ikennaokpala force-pushed the state-pension-through-partner/content-edits-21092016 branch from 21a3651 to 08b149c Compare October 13, 2016 12:56
@selfthinker
Copy link
Contributor

The title of "Correct content errors in age_dependent_pension_outcome.govspeak.erb" is now incorrect and should be changed to something like "Correct content errors in State pension through partner outcomes".

You have now removed some of the detailed change descriptions but not all of them. Is there a reason why you haven't adjusted 3. and 5. as well?

Minor: I would think the -- was only there because there was a lot of text. But when it's less text now, it could fit easily into just one list without spacing or lines inbetween.

@ikennaokpala ikennaokpala force-pushed the state-pension-through-partner/content-edits-21092016 branch 2 times, most recently from 43994f9 to 490dc18 Compare October 13, 2016 14:04
benjamin-mortimer and others added 3 commits October 13, 2016 15:05
@benjamin-mortimer @ikennaokpala
Correct content errors in State pension through partner outcomes
fe2d6d1 
This commit makes the following changes

- Correct wrong content about the rules for women who paid less National
Insurance earlier in life.
- Clarify content in sections where it was unclear. Fix several typos.

The aforementioned content changes were made to:

- age_dependent_pension_outcome.govspeak.erb
- married_woman_and_state_pension_outcome.govspeak.erb
- married_woman_no_state_pension_outcome.govspeak.erb
- widow_and_old_pension_outcome.govspeak.erb

These files are in smart-answers/lib/smart_answer_flows/state-pension-through-partner/outcomes/

The changes were requested by the Department for Work and Pensions (DWP)
in Zendesk ticket https://govuk.zendesk.com/agent/tickets/1405312.

The Trello ticket for these changes is here: https://trello.com/c/jVxRQVUn
@ikennaokpala
Update checksums data for state-pension-through-partner
cf0696f
@ikennaokpala
Regenerate regression test artefacts (SPTP)
9f94f86 
This commit holds regenerated regression test artefacts for content
changes made to State Pension Through Partner SmartAnswer.

These changes were made upon request from the, Content Team.

Already existing unit, regression and integration tests passed, giving
assurance that the aforementioned commit/changes have had no
(interfering) effect to other smart answers.
@ikennaokpala ikennaokpala force-pushed the state-pension-through-partner/content-edits-21092016 branch from 490dc18 to 9f94f86 Compare October 13, 2016 14:08
selfthinker
selfthinker approved these changes Oct 13, 2016
View reviewed changes
Copy link
Contributor

@selfthinker selfthinker left a comment

Choose a reason for hiding this comment

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

I'm happy with that, thanks @ikennaokpala. 👍
I would have added the checksum and artefact changes to the same commit but am fine with them being separate.

@ikennaokpala ikennaokpala merged commit d582918 into master Oct 13, 2016
@ikennaokpala ikennaokpala deleted the state-pension-through-partner/content-edits-21092016 branch October 13, 2016 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@selfthinker selfthinker selfthinker 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.
3 participants
@ikennaokpala @selfthinker @benjamin-mortimer