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

Generate renderers in a stable sort order #2611

Merged
merged 5 commits into from
Apr 21, 2021

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Apr 9, 2021

Also

  • Remove Annotation from @Renderers. Not necessary.
  • Tweak again which render functions are necessary.

This is a pretty huge opaque change to the renderers, but should make future changes much smaller, keeping the renderers sorted.

@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label Apr 9, 2021
@srawlins srawlins requested a review from jcollins-g April 9, 2021 17:48
@srawlins
Copy link
Member Author

@jcollins-g do you know if something has changed in the SDK to make those few tests fail? Something about releasing a 2.14 dev maybe?

@jcollins-g
Copy link
Contributor

@srawlins not sure, will take a look though. At first glance, not looking too deeply, it seems like it fails on both stable and dev SDKs so not sure the SDK would be to blame?

@srawlins
Copy link
Member Author

Lol that grinder exception has a bug:

GrinderException: expected [19, 20] dart: index.html entries, found 19

came from

  const expectedLibCounts = [0, 1];
  const expectedTotalCount = [19, 20];
  if (!expectedLibCounts.contains(foundLibs)) {
    fail(
        'expected $expectedTotalCount dart: index.html entries, found $foundLibs');
  }

@jcollins-g
Copy link
Contributor

Lol that grinder exception has a bug:

GrinderException: expected [19, 20] dart: index.html entries, found 19

came from

  const expectedLibCounts = [0, 1];
  const expectedTotalCount = [19, 20];
  if (!expectedLibCounts.contains(foundLibs)) {
    fail(
        'expected $expectedTotalCount dart: index.html entries, found $foundLibs');
  }

well it is never supposed to actually fail here :-D

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 58.859% when pulling 8a4c79d on srawlins:stable-renderers into 856c613 on dart-lang:master.

@srawlins srawlins merged commit 6efbf78 into dart-lang:master Apr 21, 2021
@srawlins srawlins deleted the stable-renderers branch April 21, 2021 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants