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

Glos confirm integration post testing fixes #293

Conversation

nephila-nacrea
Copy link
Contributor

@nephila-nacrea nephila-nacrea commented Aug 8, 2023

@nephila-nacrea nephila-nacrea self-assigned this Aug 8, 2023
@nephila-nacrea nephila-nacrea force-pushed the glos-confirm-integration-post-testing-fixes branch 2 times, most recently from 0f60223 to eb36f90 Compare August 9, 2023 08:49
@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Merging #293 (2bcba61) into glos-confirm-integration (e2b5703) will increase coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head 2bcba61 differs from pull request most recent head 1f07fd2. Consider uploading reports for the commit 1f07fd2 to get more accurate results

@@                     Coverage Diff                      @@
##           glos-confirm-integration     #293      +/-   ##
============================================================
+ Coverage                     82.67%   82.71%   +0.03%     
============================================================
  Files                           113      113              
  Lines                          3839     3841       +2     
  Branches                        578      579       +1     
============================================================
+ Hits                           3174     3177       +3     
  Misses                          413      413              
+ Partials                        252      251       -1     
Files Changed Coverage Δ
perllib/Open311/Endpoint/Integration/Confirm.pm 75.09% <100.00%> (+0.58%) ⬆️

@nephila-nacrea nephila-nacrea force-pushed the glos-confirm-integration-post-testing-fixes branch from 1a12242 to 52dbea4 Compare August 11, 2023 12:46
Copy link
Member

@davea davea left a comment

Choose a reason for hiding this comment

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

This looks fine, but I wonder if it would be better to do the string manipulation in Gloucestershire::open311_extra_data_include in FMS, given that already exists? Doing it here in open311-adapter means that there's Gloucestershire-specific functionality in two places rather than one.

Gloucestershire cobrand in fms repo sets a special location
attribute; we need to make sure it is handled in
perllib/Open311/Endpoint/Integration/Confirm.pm here.
@nephila-nacrea nephila-nacrea force-pushed the glos-confirm-integration-post-testing-fixes branch from 2bcba61 to 1f07fd2 Compare August 18, 2023 15:06
@nephila-nacrea nephila-nacrea merged commit 1f07fd2 into glos-confirm-integration Aug 18, 2023
10 checks passed
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