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

UIP-2924: Add UiPropsMapView #128

Merged

Conversation

jacehensley-wf
Copy link
Contributor

@jacehensley-wf jacehensley-wf commented Dec 17, 2017

Ultimate problem:

We should add UiPropsMapView

How it was fixed:

  • Add and test UiPropsMapView

Testing suggestions:

  • Verify tests pass

Potential areas of regression:

  • N/A only new stuff

FYA: @greglittlefield-wf @aaronlademann-wf @jacehensley-wf @clairesarsam-wf @joelleibow-wf

@aviary2-wf
Copy link

aviary2-wf commented Dec 17, 2017

Raven

Number of Findings: 0

@codecov-io
Copy link

codecov-io commented Dec 17, 2017

Codecov Report

Merging #128 into master will increase coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
+ Coverage   94.44%   94.48%   +0.05%     
==========================================
  Files          32       33       +1     
  Lines        1581     1593      +12     
==========================================
+ Hits         1493     1505      +12     
  Misses         88       88

Copy link
Contributor

@aaronlademann-wf aaronlademann-wf left a comment

Choose a reason for hiding this comment

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

+1

@jacehensley-wf should we add a tech-debt follow-up ticket so we remember to remove this from WSD once this is released?

@jacehensley-wf
Copy link
Contributor Author

Yup, I can create a follow-up ticket

@jacehensley-wf jacehensley-wf changed the title Add UiPropsMixinMapView UIP-2924: Add UiPropsMixinMapView Dec 18, 2017
Copy link
Contributor

@greglittlefield-wf greglittlefield-wf left a comment

Choose a reason for hiding this comment

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

When pulled into WSD, I get the following error:

error: The name 'UiPropsMixinMapView' is defined in the libraries 'react_util.dart' and 'react_util.dart'. (ambiguous_export at [web_skin_dart] lib/ui_core.dart:12)

which causes a bunch of other analyzer errors.

The error message is misleading, since both WSD and over_react files are named react_util.dart; the problem is that both lines try to export that class:

export 'package:over_react/over_react.dart';
export 'src/ui_core/util/react_util.dart';

I'm not sure there's a way to export this class in the main over_react entrypoint in a way that's compatible with WSD.

@jacehensley-wf jacehensley-wf changed the title UIP-2924: Add UiPropsMixinMapView UIP-2924: Add UiPropsMapView Jan 3, 2018
@aaronlademann-wf
Copy link
Contributor

+1

@aaronlademann-wf
Copy link
Contributor

QA +10

  • Testing instruction
  • Dev +1's
  • Dev/QA +10
  • Unit tests created/updated
  • All unit tests pass
  • Rosie ran/Rosie comment displays expected info
  • Dependency Scan Clean

Merging.

@aaronlademann-wf aaronlademann-wf merged commit fc17df5 into Workiva:master Jan 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants