search

cdli-gh / cdli-mobile-app

MIT License
4 stars 8 forks source link

Resolves Issue #9 (warnings and typos removed) #11

Open kanavpruthi opened 3 years ago

kanavpruthi commented 3 years ago

1.) Removed all warnings throughout the project. 2.) Removed unused import statements 3.)Commented out unused variables but not deleted them, thus saving them for future use.

NishealJ commented 3 years ago

@kanavpruthi Can you resolve the conflict? Thanks!

kanavpruthi commented 3 years ago

Hey @NishealJ, thanks for the update on this request! The conflicts have been resolved and it's ready to merge.

NishealJ commented 3 years ago

@kanavpruthi PR has changed pubslock looks unintentional also please check the manifest.java & R.java added by your IDE.

kanavpruthi commented 3 years ago

Thanks, @NishealJ , Android Studio auto-updated the versions of some of the dependencies used in the project, I have reverted back to the original settings, please check!

kanavpruthi commented 3 years ago

Please confirm the acceptance of this PR @NishealJ , since I have already started working on another issue on this project currently, thanks :)

NishealJ commented 3 years ago

Thanks @kanavpruthi I'll resolve this PR by mid-week Thanks!

NishealJ commented 3 years ago

@MrUnfunny can you help in testing PR?

MrUnfunny commented 3 years ago

@NishealJ yeah, sure

kanavpruthi commented 3 years ago

Hey @MrUnfunny , I am really unsure how I missed that, but thanks for informing. I have fixed that right now, please test it again!

kanavpruthi commented 3 years ago

Hey @MrUnfunny, my commit shows that the required changes have been made, and I don't know why this error seems to persist. Also, the reason was to name the classes on the basis of Dart's recommended Camel cases and remove all warnings(hence the return statement, since it won't affect the functionality of the callback function). Thanks! I've also made a minor change to how the logo currently looks because it previously had a white background in an android device, I've made a small modification and it looks much better at the moment. Please do check!

kanavpruthi commented 3 years ago

Hey @MrUnfunny, please check the latest PR, the app is running fine on the system, with all warnings/typos removed and minor modification to the existing logo, please check. Thanks!

MrUnfunny commented 3 years ago

Hi @kanavpruthi All the functionalities work fine on my machine but I think logo modifications must be a seperate PR and you should revert onWillPop: () { _onBackPressed(context); return; } back to onWillPop: () => _onBackPressed(context) as the latter is less verbose and more readable.

kanavpruthi commented 3 years ago

Hey @MrUnfunny, I have reverted the original logo changes as of now, and I still believe we should write the code this way because of Dart's guidelines, it leaves a warning otherwise, please consider!

kanavpruthi commented 3 years ago

Hey @MrUnfunny @NishealJ, I have opened an issue regarding the logo and have reverted changes here.