Skip to content

Feat: Added feature(in shipmentsDetailsOffline package) to get tracki…#35

Open
Photon3009 wants to merge 33 commits intoshankarpriyank:masterfrom
Photon3009:Feat_OfflineMode
Open

Feat: Added feature(in shipmentsDetailsOffline package) to get tracki…#35
Photon3009 wants to merge 33 commits intoshankarpriyank:masterfrom
Photon3009:Feat_OfflineMode

Conversation

@Photon3009
Copy link
Copy Markdown
Contributor

…ng link via SMS body, for now, it is for Flipkart, and for now, we are just showing results in Logcat

…ng link via SMS body, for now it is for flipkart and for now we are just showing results in Logcat
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

It's great having you contribute to this project

Welcome to the open-source world 🤓

It is really good to see that you have taken out your precious time to contribute to this project. Communication is very important for open-source,please try to communicate well. Any questions?let me know!

…changes in trackshipmentViewmodel and other files
Copy link
Copy Markdown
Owner

@shankarpriyank shankarpriyank left a comment

Choose a reason for hiding this comment

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

Please avoid pushing the changes that are not necessary.
We are making progress, but there is still quite some work to be done.
Also please have a look at why the CodeFactor check is also failing

import androidx.lifecycle.ViewModel
import com.priyank.drdelivery.offlineShipmentDetails.Domain.model.RequiredSMS

class SMSViewModel() : ViewModel() {
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.

No need of a new viewmodel

data class RequiredSMS(
val id: Int,
val address: String,
val date: String,
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.

The variable names can be more descriptive

build.gradle Outdated
}
dependencies {
classpath("com.google.dagger:hilt-android-gradle-plugin:2.38.1")
classpath('com.google.dagger:hilt-android-gradle-plugin:2.40.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.

unnecessary changes, please undo them

build.gradle Outdated
id 'com.android.application' version '7.1.2' apply false
id 'com.android.library' version '7.1.2' apply false
id 'com.android.application' version '7.2.2' apply false
id 'com.android.library' version '7.2.2' apply false
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.

same as above

#Wed Apr 06 20:56:10 IST 2022
distributionBase=GRADLE_USER_HOME
distributionUrl=https\://services.gradle.org/distributions/gradle-7.2-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-7.3.3-bin.zip
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.

unnecessary

Copy link
Copy Markdown
Owner

@shankarpriyank shankarpriyank left a comment

Choose a reason for hiding this comment

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

You should not delete the gitignore file, also please comment on github itself and try to write better commit messages

build.gradle Outdated
}
dependencies {
classpath("com.google.dagger:hilt-android-gradle-plugin:2.38.1")
classpath('com.google.dagger:hilt-android-gradle-plugin:2.38.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.

unnecessary changes

@shankarpriyank
Copy link
Copy Markdown
Owner

Also have a look at the issues identified by CodeFactor

private val perDetails: PerDetails = mockk(relaxed = true)

@Test
fun `on init with user logged in, should update start destination state to screen detail`() = runTest {
Copy link
Copy Markdown
Owner

@shankarpriyank shankarpriyank Jan 27, 2023

Choose a reason for hiding this comment

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

HI,@minibugdev This test is failing, we have changed added a feature to use the application without authenticating the user by using Gmail API
Any suggestions how should we go about refactoring this test?

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