Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(146)

Issue 1325053002: - Remove scanning by regular expression and redundant scanning. (Closed)

Created:
5 years, 3 months ago by Ivan Posva
Modified:
4 years, 8 months ago
Reviewers:
nweiz, Bob Nystrom
CC:
reviews_dartlang.org
Base URL:
git@github.com:dart-lang/yaml.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

- Remove scanning by regular expression and redundant scanning.

Patch Set 1 #

Patch Set 2 : Updated comments #

Patch Set 3 : Fix checked mode. #

Patch Set 4 : Updated to df28edda6846b052d75aea0585b8d8aa250d96f2 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+173 lines, -93 lines) Patch
M lib/src/loader.dart View 1 2 3 3 chunks +173 lines, -93 lines 0 comments Download

Messages

Total messages: 10 (2 generated)
Ivan Posva
5 years, 3 months ago (2015-09-02 06:26:31 UTC) #2
Bob Nystrom
Thank you for doing this! I'll hand this off to Natalie since YAML is mostly ...
5 years, 3 months ago (2015-09-02 16:28:02 UTC) #4
nweiz
On 2015/09/02 16:28:02, Bob Nystrom wrote: > Thank you for doing this! > > I'll ...
5 years, 3 months ago (2015-09-02 18:56:23 UTC) #5
Ivan Posva
On 2015/09/02 18:56:23, nweiz wrote: > On 2015/09/02 16:28:02, Bob Nystrom wrote: > > Thank ...
5 years, 3 months ago (2015-09-02 19:19:04 UTC) #6
nweiz
On 2015/09/02 19:19:04, Ivan Posva wrote: > On 2015/09/02 18:56:23, nweiz wrote: > > On ...
5 years, 3 months ago (2015-09-02 20:22:41 UTC) #7
Ivan Posva
On 2015/09/02 20:22:41, nweiz wrote: > On 2015/09/02 19:19:04, Ivan Posva wrote: > > On ...
5 years, 3 months ago (2015-09-02 20:59:13 UTC) #8
nweiz
On 2015/09/02 20:59:13, Ivan Posva wrote: > On 2015/09/02 20:22:41, nweiz wrote: > > On ...
5 years, 3 months ago (2015-09-02 21:17:25 UTC) #9
Ivan Posva
5 years, 3 months ago (2015-09-02 21:32:23 UTC) #10
On 2015/09/02 21:17:25, nweiz wrote:
> On 2015/09/02 20:59:13, Ivan Posva wrote:
> > On 2015/09/02 20:22:41, nweiz wrote:
> > > On 2015/09/02 19:19:04, Ivan Posva wrote:
> > > > On 2015/09/02 18:56:23, nweiz wrote:
> > > > > On 2015/09/02 16:28:02, Bob Nystrom wrote:
> > > > > > Thank you for doing this!
> > > > > > 
> > > > > > I'll hand this off to Natalie since YAML is mostly her baby. This is
> > going
> > > > to
> > > > > > need a bunch of style changes, do you want to do those, or should we
> > just
> > > > take
> > > > > > it from here?
> > > > > 
> > > > > It looks like a bunch of the tests don't pass with this.
> > > > 
> > > > Care to be a tiny bit more specific? I ran all the tests I could find in
> the
> > > > yaml repo, which are yaml_test.dart and yaml_node_wrapper_test.dart, and
> > they
> > > > are green all the way.
> > > > 
> > > > Thanks,
> > > > -Ivan
> > > 
> > > When I patch this in to a080e6b8b0b2609858a755797386079c20c2aa02 (the last
> > > revision it merges cleanly with), run "pub upgrade", and run "pub run
test"
> I
> > > get this output: https://gist.github.com/nex3/e35aedbb2900512a6a8d
> > 
> > Thanks! Should have also tested checked mode...
> > 
> > "pub run test" now works cleanly.
> > 
> > -Ivan
> 
> Awesome, thanks! I'll take it over from here.

Updated to ToT now.

-Ivan

Powered by Google App Engine
This is Rietveld 408576698