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

AF-1899 Dart 2 compatibility #320

Merged
merged 10 commits into from
Oct 10, 2018
Merged

AF-1899 Dart 2 compatibility #320

merged 10 commits into from
Oct 10, 2018

Conversation

evanweible-wf
Copy link
Contributor

Description

Update w_transport to be compatible with Dart 2 and make sure CI is running on both Dart 1 and 2 (dev).

Testing

  • CI passes (dart 1.24.3 and dev channel)

Code Review

@Workiva/app-frameworks
@robbecker-wf

@evanweible-wf evanweible-wf requested a review from a team as a code owner August 1, 2018 03:19
@aviary2-wf
Copy link

aviary2-wf commented Aug 1, 2018

Security Insights

No security relevant content was detected by automated scans.

Action Items

  • Review PR for security impact; comment "security review required" if needed or unsure
  • Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on HipChat: InfoSec Forum.

@evanweible-wf evanweible-wf force-pushed the dart2 branch 3 times, most recently from 06b01db to a24c87c Compare August 1, 2018 03:27
@evanweible-wf evanweible-wf force-pushed the dart2 branch 3 times, most recently from 0ee48f0 to bdb6d3b Compare August 1, 2018 20:52
.travis.yml Outdated
@@ -17,33 +17,34 @@ stages:
jobs:
include:
- stage: quality
dart: stable
Copy link
Member

@robbecker-wf robbecker-wf Oct 3, 2018

Choose a reason for hiding this comment

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

let's include 1.24.3 and stable

pubspec.yaml Outdated
@@ -28,29 +28,24 @@ dependencies:
sockjs_client:
git:
url: git://github.com/Workiva/sockjs-dart-client.git
ref: 0.3.3
sockjs_client_wrapper: ^1.0.4
ref: 0.3.4
Copy link
Member

Choose a reason for hiding this comment

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

sockjs_client 0.3.5 is released that is dart2 compatible.

pubspec.yaml Outdated
ref: 0.3.3
sockjs_client_wrapper: ^1.0.4
ref: 0.3.4
sockjs_client_wrapper: ^1.0.5

dev_dependencies:
browser: ^0.10.0+2
Copy link
Member

@robbecker-wf robbecker-wf Oct 3, 2018

Choose a reason for hiding this comment

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

need to remove browser and replace .dart with .dart.js in html files to load the compiled JS always.
html files to update:
image

@robbecker-wf
Copy link
Member

So close to ready for dart 2. Just need a few fixes and a dart 2 compatible dart_dev and were there.

@evanweible-wf evanweible-wf force-pushed the dart2 branch 2 times, most recently from 5f0dee4 to 9cc5513 Compare October 8, 2018 16:44
Dart 2: TBD

Copy link
Member

Choose a reason for hiding this comment

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

Can we just say pub run build_runner serve example here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but we will also need to upgrade to the version of over_react that eventually includes builder support. That's mainly why I was leaving this as TBD since the examples just won't work on Dart 2 right now

Copy link
Contributor

Choose a reason for hiding this comment

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

imo, we should create a ticket for this change so it doesn't get lost in the dart 2 work.

Dart 2: TBD

Copy link
Contributor

Choose a reason for hiding this comment

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

imo, we should create a ticket for this change so it doesn't get lost in the dart 2 work.

Copy link
Member

@robbecker-wf robbecker-wf left a comment

Choose a reason for hiding this comment

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

Looking good!

@robbecker-wf
Copy link
Member

+10 CI passes under dart 1 and dart 2

@robbecker-wf
Copy link
Member

Quality Review Approval: +1 ( aka QA: +1 )

  • Testing instruction
  • Dev +1's
  • Dev/QA +10 with detail of what was tested
  • Unit test created/updated
  • All unit tests pass
  • Rosie has run and reports properly the release the ticket will be included in

Merging into master.
@Workiva/release-management-p for merge into master

@rm-astro-wf rm-astro-wf merged commit 1ec7a50 into master Oct 10, 2018
@rm-astro-wf rm-astro-wf deleted the dart2 branch October 10, 2018 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants