-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Dart 3 null safety #1213
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
base: master
Are you sure you want to change the base?
Dart 3 null safety #1213
Changes from all commits
79d7365
883c870
a73142b
f945fd9
4351249
e03c793
5f62e8e
393e758
fad0cab
4d7f205
b153dfb
c65ac96
b608117
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| @use "sass:math"; | ||
|
|
||
| article.chapter { | ||
| h2 { | ||
| font: 600 30px/24px $serif; | ||
|
|
@@ -192,8 +194,8 @@ article.chapter { | |
| @media only screen and (max-width: 630px) { | ||
| article.chapter { | ||
| h2 a::before, h3 a::before { | ||
| left: -($col / 2); | ||
| width: $col / 2; | ||
| left: -(math.div($col, 2)); | ||
| width: math.div($col, 2); | ||
|
Comment on lines
+197
to
+198
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: if possible, would prefer this PR to have been split into multiple parts instead of all boilerplate in 1 (thought not sure if that's possible). Ex:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I kind of felt the same way about this one. It was mixed. I can work on splitting this up into the dart pacakges as one update, and the dart 3 in another. I can't remember if I started getting errors on one of the packages and that is why I went ahead and updated them, or if this just started to feel like a modernization story to me and I updated the packages. These were just warnings that they would break in the future at any rate. I try not to have warnings emitted with a build. But this isn't really related to the nullability safety at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, thanks for context. I feel you, thanks for fixing them. Since I've already begun reviewing I'm cool with continuing, just a heads up for future reference! |
||
| } | ||
| } | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is not required to unblock the main issue (Dart 3 dep), this should be in it's own separate PR. Being able to run this requires installing
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I based this PR onto my other PR which pinned the dart version at dart 2. Being able to have a locked down version of DART was kind of key to being able to validate this migration, because I could then compare the output of the dart2 to the output of dart3, and make sure I didn't accidentally introduce a regression. I know a lot of folks don't use nix, but this is why it was there. I had made the prior PR, because it would only build under dart2, and I had to spend a bit of time figuring out how to get an old version up and going. nix package manager is supported on many distros. Using a dockerfile could get a similar experience, and maybe be useful to more people. But I use nix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for context, sure I'm ok with adding it since it's additive |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| # Using the NixOS 25.11 stable release for a consistent Dart 3 environment. | ||
| { pkgs ? import (fetchTarball { | ||
| url = "https://github.com/NixOS/nixpkgs/archive/nixos-25.11.tar.gz"; | ||
| sha256 = "1hp1ddh41wrqxgrnfa79nsv7gzlbpwcqsijx7jw5b33wvk5ah5gb"; | ||
| }) {} }: | ||
|
|
||
| pkgs.mkShell { | ||
| buildInputs = [ | ||
| pkgs.dart | ||
| pkgs.zulu | ||
| pkgs.gcc | ||
| pkgs.gnumake | ||
| pkgs.git | ||
| pkgs.glibcLocales | ||
| ]; | ||
|
|
||
| shellHook = '' | ||
| export LANG=en_US.UTF-8 | ||
| ''; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On an early version of dart3, it will build fine without this. Prior to pushing this, I updated to a more recent dart 3, and the build started hanging with an OOM error. Adding this flag allowed it to once again build successfully. You could try with
DART_FLAGS := ""and see if you can replicate it, or if it is needed. I don't use dart a whole lot, so I'm not sure if this was maybe something transient introduce in one version and then fixed, or if it will carry forward to future versions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it thanks, can we add 1-line code comment so this is easier for people in future to know why it was set?