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.

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

Remove Germany logic from marriage-abroad outcome_os_consular_cni #2352

Conversation

Projects
None yet
2 participants
@chrisroos
Copy link
Contributor

commented Mar 10, 2016

In 0184143, information about opposite sex marriages in Germany was moved from the outcome_os_consular_cni outcome to outcome_os_germany. This branch removes the now unused code from outcome_os_consular_cni.

@floehopper floehopper self-assigned this Mar 11, 2016

@floehopper

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2016

Ideally there'd be a reference to the point at which outcome_os_germany was introduced i.e. where this code should've been removed. However, don't worry if it's hard to find because of the v2 git history craziness.

Otherwise this looks good to me. 👍

chrisroos added some commits Mar 10, 2016

Remove unreachable blocks of code
Opposite sex marriages in Germany have their own outcome
(outcome_os_germany) which means that `ceremony_country` will never be
"germany" in this outcome.
Inline :ceremony_not_germany_or_not_resident_other
I'm removing the code that deals with a ceremony country of Germany from
the outcome_os_consular_cni template. Inlining this `precalculate` block
makes it clearer that it needs to change.
Remove Germany from list of countries
Opposite sex marriages in Germany have their own outcome
(outcome_os_germany) which means that `ceremony_country` will never be
"germany" in this outcome.
Remove unnecessary conditions around code blocks
Opposite sex marriages in Germany have their own outcome
(outcome_os_germany) which means that `ceremony_country` will never be
"germany" in this outcome.

I've removed these unecessary conditions and de-indented the inner
blocks of code where necessary. Use `git show --ignore-all-space` to see
the diff more clearly.
Remove unnecessary conditions
Opposite sex marriages in Germany have their own outcome
(outcome_os_germany) which means that `ceremony_country` will never be
"germany" in this outcome.
Update checksum data for marriage-abroad
The marriage-abroad regression tests are passing so I've updated the
checksum data.

Updated using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
    marriage-abroad
@chrisroos

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2016

Thanks for reviewing this, @floehopper. I've updated the PR description with a link to the commit that added the outcome_os_germany outcome.

@chrisroos chrisroos force-pushed the remove-germany-logic-from-marriage-abroad-outcome-os-consular-cni branch from d865b8d to d635112 Mar 18, 2016

@chrisroos

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2016

I've rebased this on master and force pushed now that PR #2349 (Remove Japan from marriage-abroad outcome_os_consular_cni) has been merged.

I'm going to wait for the tests to pass and get it merged.

chrisroos added a commit that referenced this pull request Mar 18, 2016

Merge pull request #2352 from alphagov/remove-germany-logic-from-marr…
…iage-abroad-outcome-os-consular-cni

Remove Germany logic from marriage-abroad outcome_os_consular_cni

@chrisroos chrisroos merged commit c559092 into master Mar 18, 2016

1 check passed

default "Build #4505 succeeded on Jenkins"
Details

@chrisroos chrisroos deleted the remove-germany-logic-from-marriage-abroad-outcome-os-consular-cni branch Mar 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.