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

fix: remove relative component of import #3332

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Oct 6, 2023

Reported with a proposed fix in #3331, but that can't run all the necessary CI checks

#skip-changelog

@armcknight
Copy link
Member Author

@vaind Would you mind reviewing this one? I'm not sure why the import was originally written that way. Is this going to break the Dart SDK somehow?

@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #3332 (0404e34) into main (c2acec5) will decrease coverage by 0.059%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3332       +/-   ##
=============================================
- Coverage   89.271%   89.213%   -0.059%     
=============================================
  Files          501       501               
  Lines        54222     54217        -5     
  Branches     19481     19481               
=============================================
- Hits         48405     48369       -36     
- Misses        4962      4986       +24     
- Partials       855       862        +7     

see 12 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2acec5...0404e34. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1227.71 ms 1248.70 ms 20.99 ms
Size 22.85 KiB 408.88 KiB 386.04 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
5e66a38 1209.10 ms 1233.90 ms 24.79 ms
cd39d58 1203.87 ms 1239.88 ms 36.01 ms
f8fc36d 1226.31 ms 1247.80 ms 21.49 ms
102f2a6 1225.71 ms 1244.28 ms 18.57 ms
db31083 1227.69 ms 1243.56 ms 15.87 ms
7419285 1209.53 ms 1244.72 ms 35.19 ms
c6773e5 1222.48 ms 1240.02 ms 17.54 ms
443723a 1205.24 ms 1220.52 ms 15.28 ms
a2af9fa 1236.29 ms 1251.67 ms 15.38 ms
8f397a7 1219.12 ms 1236.67 ms 17.55 ms

App size

Revision Plain With Sentry Diff
5e66a38 22.85 KiB 408.88 KiB 386.03 KiB
cd39d58 20.76 KiB 435.26 KiB 414.50 KiB
f8fc36d 20.76 KiB 419.70 KiB 398.94 KiB
102f2a6 20.76 KiB 433.18 KiB 412.42 KiB
db31083 22.85 KiB 407.63 KiB 384.78 KiB
7419285 20.76 KiB 432.99 KiB 412.22 KiB
c6773e5 20.76 KiB 435.25 KiB 414.49 KiB
443723a 20.76 KiB 414.44 KiB 393.68 KiB
a2af9fa 20.76 KiB 432.88 KiB 412.11 KiB
8f397a7 20.76 KiB 420.55 KiB 399.79 KiB

Copy link
Collaborator

@vaind vaind left a comment

Choose a reason for hiding this comment

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

@vaind Would you mind reviewing this one? I'm not sure why the import was originally written that way. Is this going to break the Dart SDK somehow?

I don't think there was any specific reason for this - just using the actual path to the import file. Maybe it was even added automatically 🤷
Also, I don't think this would have any impact on Dart because this is an internal file

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM

@armcknight
Copy link
Member Author

Thanks for confirming @vaind and for suggesting the original change @jboulter11!

@armcknight armcknight merged commit 39b1c35 into main Oct 6, 2023
66 of 67 checks passed
@armcknight armcknight deleted the armcknight/fix/import-path branch October 6, 2023 18:02
@jboulter11
Copy link

Thank you very much for getting this taken care of for me. 😄

philipphofmann pushed a commit that referenced this pull request Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants