Skip to content

Onboard to Dart null safety#11

Open
asimihsan wants to merge 1 commit intodnfield:masterfrom
asimihsan:master
Open

Onboard to Dart null safety#11
asimihsan wants to merge 1 commit intodnfield:masterfrom
asimihsan:master

Conversation

@asimihsan
Copy link
Copy Markdown

In this commit we onboard date_calendar to Dart null safety. This will
allow other Flutter projects and Dart packages that depend on
date_calendar to themselves onboard to Dart null safety.

For more information please see [1].

Unfortunately I had to relax one of the analysis options, since I used
casting to get out of some errors.

Tested by running flutter analyze and flutter test.

[1] https://dart.dev/null-safety

In this commit we onboard date_calendar to Dart null safety. This will
allow other Flutter projects and Dart packages that depend on
date_calendar to themselves onboard to Dart null safety.

For more information please see [1].

Unfortunately I had to relax one of the analysis options, since I used
casting to get out of some errors.

Tested by running `flutter analyze` and `flutter test`.

[1] https://dart.dev/null-safety
@asimihsan asimihsan mentioned this pull request Jun 20, 2021
class GregorianCalendar implements Calendar {
/// Setting any of these to null treats them as 1 or 0 (year)
GregorianCalendar(int year, [int month = 1, int day = 1]) {
GregorianCalendar(this._year, [int? month = 1, int? day = 1]) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think it'd be better to just make these non-null.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I agree, for this constructor despite the comment on line 50 Setting any of these to null treats them as 1 or 0 (year) you should assume these are non-null.

Copy link
Copy Markdown
Owner

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

I've started looking through this, and I have a number of comments along the lines of "this should probably be non-null."

If you're interested in working through that with me, I'm happy to do so, though it may be faster for me to just migrate this library myself :). Let me know.

@asimihsan
Copy link
Copy Markdown
Author

asimihsan commented Jul 2, 2021

I apologize for my late reply...I haven't collaborated on GitHub in a long time.

If you're interested in working through that with me, I'm happy to do so, though it may be faster for me to just migrate this library myself :). Let me know.

I don't mind either way. This library is crucial to a Flutter app I made for my wife so I feel a sense of gratitude to you. I don't need credit for this pull request, but if I can help you in any way let me know. If it's faster for you to migrate it go for it!

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.

2 participants