pariyatti / mobile-app

The Pariyatti mobile app
https://pariyatti.app/
GNU Affero General Public License v3.0
8 stars 11 forks source link

issue 12 solved #17

Closed jHetvi closed 3 years ago

jHetvi commented 3 years ago

In pubspec.lock https://github.com/pariyatti/patta/pull/17#discussion_r470139243:

@@ -218,6 +225,13 @@ packages: url: "https://pub.dartlang.org" source: hosted version: "1.1.1"

  • fake_async:

I'm not sure what this new dependency (fake_async) is. Is it necessary?

In pubspec.lock https://github.com/pariyatti/patta/pull/17#discussion_r470139880:

@@ -113,13 +113,20 @@ packages: url: "https://pub.dartlang.org" source: hosted version: "2.1.0+1"

  • characters:

I'm not sure what this new dependency (characters) is. Is it necessary?

Even I don't know about this new changes as it's coming after I updated my Flutter to the latest version. But I'll try an research and let you know about these changes as soon as I learn about it.

And for the rest yes I will make the required changes and commit again.

On Thu, 13 Aug 2020 at 23:34, Steven Deobald notifications@github.com wrote:

@deobald requested changes on this pull request.

One general comment: It looks like your text editor removed all the TODO: text from the TODO comments... could you turn that setting off and replace the TODO:s? We use those as markers that are easy to search for when we're looking for the small tasks that have emerged in the code.

I've made a few other comments on the files you've submitted. Your changes are looking pretty good! There are just a few things to double-check or correct and then we should be able to accept this PR.

Oh! One last note: You'll need to "sign" (digitally is fine) the Contributor Agreement:

https://docs.google.com/document/d/14GymHIfYAWZy3kIqEIvqyIDPoi6lNwE0AQ8rdd9iuUk/edit

...you can just make a copy of that file, add your changes to the bottom, and send me the updated file or print it to PDF and send me that. Thanks!

In pubspec.lock https://github.com/pariyatti/patta/pull/17#discussion_r470139243:

@@ -218,6 +225,13 @@ packages: url: "https://pub.dartlang.org" source: hosted version: "1.1.1"

  • fake_async:

I'm not sure what this new dependency (fake_async) is. Is it necessary?

In pubspec.lock https://github.com/pariyatti/patta/pull/17#discussion_r470139880:

@@ -113,13 +113,20 @@ packages: url: "https://pub.dartlang.org" source: hosted version: "2.1.0+1"

  • characters:

I'm not sure what this new dependency (characters) is. Is it necessary?

In .vscode/launch.json https://github.com/pariyatti/patta/pull/17#discussion_r470141516:

@@ -0,0 +1,14 @@ +{

  • // Use IntelliSense to learn about possible attributes.
  • // Hover to view descriptions of existing attributes.
  • // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
  • "version": "0.2.0",
  • "configurations": [
  • {
  • "name": "Flutter",
  • "program": "lib/main_sand.dart",
  • "request": "launch",
  • "type": "dart"
  • }
  • ] +}

I'm guessing this is necessary for most people who are editing the project with VS Code? I think it might be a better option to git-ignore the /.vscode directory instead, but this is probably worth a discussion in

hackathon-flutter-pariyatti with one of the mentors who uses VS Code on a

daily basis. :)

In lib/ui/screens/HomeScreen.dart https://github.com/pariyatti/patta/pull/17#discussion_r470144630:

@@ -51,11 +51,17 @@ class _HomeScreenState extends State { Icons.today, color: Color(0xff6d695f), ),

  • activeIcon: Icon(
  • Icons.today,
  • color: Colors.brown,

It might seem kind of finicky, but the colour in the wireframes isn't exactly brown. If you use an eyedropper tool in an image editing app you'll see the colour is RGB = 186, 86, 38 or a hex code of #ba5626. We should probably use the exact colour the designers had chosen.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pariyatti/patta/pull/17#pullrequestreview-467000176, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANVNFNQ5O5NJJAML2L2Q6F3SAQTLPANCNFSM4P6VN4RQ .

VarunBarad commented 3 years ago

@jHetvi @deobald don't bother with changes in pubspec.lock as long as no changes have been made in pubspec.yaml

@deobald ideally pubspec.lock should be ignored from VCS, I did not know about it when I was working on that project. Once this PR is merged in, I will add pubspec.lock to .gitignore and remove it from tracking.

deobald commented 3 years ago

Closing this PR because it's accidentally against the master branch in pariyatt/patta.