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.

Sign up for GitHub

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

Jump to bottom

Added null safety #65

Closed
wants to merge 1 commit into from
Closed

Added null safety #65

wants to merge 1 commit into from

Conversation

Samdid
Copy link
Contributor

@Samdid Samdid commented Mar 26, 2021

Related Issue

  • Null Safety - I have to migrate the current non-nullable project to the nullable project

Closes: #37

Describe the changes you've made

I have migrated the non-nullable project to nullable project by upgrading the SDK version and some packages. Some packages were non-compatible to null safety, then I have upgraded them by downloading the latest version of that package which is compatible to null safety. Also, changed some code manually to make the project nullable.

Type of change

What sort of change have you made:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, local variables)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Mention any unusual behaviour of your code (Write NA if not)

NA

Checklist:

  • My code follows the guidelines of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly whereever it was hard to understand.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.

Additional Info (optional)

None

@Samdid
Copy link
Contributor Author

Samdid commented Mar 26, 2021

@shagun25 I have migrated the complete project, please review and merge this PR.

@hiashutoshsingh hiashutoshsingh added gssoc21 Level2 Bug fixing, adding small features. labels Mar 26, 2021
hiashutoshsingh
hiashutoshsingh requested changes Mar 26, 2021
View reviewed changes
Copy link
Collaborator

@hiashutoshsingh hiashutoshsingh left a comment

Choose a reason for hiding this comment

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

Hey, @Samdid thanks for submitting PR.
This PR needs a lot of changes.

  1. I can see a change on 2 files only, the rest files should adhere to null safety.
  2. can't see the pubspec.yaml changes for packages.
  3. unnecessary cli folder is added by mistake

Please resolve these changes, I am sure you will able to pull this off.
Let me know if you face any blocker.

Sheheroes_WatchOS/pubspec.lock
@@ -0,0 +1,852 @@
# Generated by pub
# See https://dart.dev/tools/pub/glossary#lockfile
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove this lock file from commit, but keep this in your local.
See Why: https://dart.dev/guides/libraries/private-files#pubspeclock

my_cli/.gitignore
@@ -0,0 +1,9 @@
# Files and directories created by pub
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove cli folder, there is no need for this I guess.

my_cli/CHANGELOG.md
@@ -0,0 +1,3 @@
## 1.0.0

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove cli folder.

my_cli/README.md
@@ -0,0 +1,5 @@
A sample command-line application with an entrypoint in `bin/`, library code
in `lib/`, and example unit test in `test/`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove cli folder.

my_cli/analysis_options.yaml
@@ -0,0 +1,14 @@
# Defines a default set of lint rules enforced for
# projects at Google. For details and rationale,
# see https://github.com/dart-lang/pedantic#enabled-lints.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove cli folder.

my_cli/lib/my_cli.dart
@@ -0,0 +1,3 @@
int calculate() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove cli folder.

my_cli/pubspec.yaml
@@ -0,0 +1,14 @@
name: my_cli
description: A sample command-line application.
# version: 1.0.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove cli folder.

my_cli/pubspec.lock
@@ -0,0 +1,334 @@
# Generated by pub
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove cli folder.

my_cli/test/my_cli_test.dart
@@ -0,0 +1,8 @@
import 'package:my_cli/my_cli.dart';
import 'package:test/test.dart';

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove cli folder.

pubspec.lock
@@ -0,0 +1,992 @@
# Generated by pub
# See https://dart.dev/tools/pub/glossary#lockfile
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove this lock file from commit, but keep this in your local.
See Why: https://dart.dev/guides/libraries/private-files#pubspeclock

@Samdid
Copy link
Contributor Author

Samdid commented Mar 26, 2021

@hiashutoshsingh I will resolve all the changes asap

@hiashutoshsingh
Copy link
Collaborator

Hey @Samdid what's the update on the above ones, this is pending for a long time. Let's try to resolve all commented issues in 2-3 days, I am sure you will be able to pull this off.

@Samdid
Copy link
Contributor Author

Samdid commented Apr 5, 2021

Hey @Samdid what's the update on the above ones, this is pending for a long time. Let's try to resolve all commented issues in 2-3 days, I am sure you will be able to pull this off.

Okay

@hiashutoshsingh
Copy link
Collaborator

Hey @Samdid what's the update on the above ones, this is pending for a long time. Let's try to resolve all commented issues in 2-3 days, I am sure you will be able to pull this off.

Okay

Hey @Samdid what is the status of the above?

@hiashutoshsingh
Copy link
Collaborator

Hey @Samdid what's the update on the above ones, this is pending for a long time. Let's try to resolve all commented issues in 2-3 days, I am sure you will be able to pull this off.

any updates on the above will be closing this PR, wanted to give chance to others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hiashutoshsingh hiashutoshsingh hiashutoshsingh requested changes

Assignees
No one assigned
Labels
gssoc21 Level2 Bug fixing, adding small features.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Null safety
2 participants
@Samdid @hiashutoshsingh