Skip to content

Misc cleanup#630

Merged
rm-astro-wf merged 3 commits intomasterfrom
greglittlefield-wf-patch-3
Oct 5, 2020
Merged

Misc cleanup#630
rm-astro-wf merged 3 commits intomasterfrom
greglittlefield-wf-patch-3

Conversation

@greglittlefield-wf
Copy link
Contributor

@greglittlefield-wf greglittlefield-wf commented Sep 30, 2020

Motivation

  1. When CI fails due to dart2js test failures, it's hard to see the stack of what failed since everything is minified.
  2. Our to-do example app doesn't have ignores of ungenerated URIs set up, so it's not a valid representation of what most of our consumers use

Changes

  1. Disable minification in dart2js builds of tests/examples
  2. Pull in workiva_analysis_options to ignore ungenerated URI warnings
    • Also remove redundant pedantic lints

Release Notes

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:

QA Checklist

  • CI passes (excluding dev build of Dart SDK, which is failing for a known regression)

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

  • A Client Platform member has reviewed these changes
  • There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
  • For release PRs - Version metadata in Rosie comment is correct

@aviary3-wk
Copy link

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 Slack: #support-infosec.

@greglittlefield-wf greglittlefield-wf marked this pull request as ready for review September 30, 2020 21:27
@greglittlefield-wf greglittlefield-wf changed the title Disable dart2js minification for better debugging Misc cleanup Oct 5, 2020
Copy link
Contributor

@sydneyjodon-wk sydneyjodon-wk left a comment

Choose a reason for hiding this comment

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

+10

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.

Looks good!

@robbecker-wf
Copy link
Member

+10 verified that the analysis errors are not present in 2.7.2 and are in 2.10 so Google can reproduce our.upgrade issues.

@greglittlefield-wf
Copy link
Contributor Author

@Workiva/release-management-p

Copy link

@rmconsole-wf rmconsole-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 from RM

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.

7 participants