Skip to content

Conversation

@vigneshTheDev
Copy link
Collaborator

Implements the task in the issue: #2970

@vikasrohit
Copy link

Overall looks good with few suggestions.

@vikasrohit
Copy link

@vigneshTheDev Do you think you would be able to wrap the suggested changes today EOD?

Copy link
Collaborator Author

@vigneshTheDev vigneshTheDev left a comment

Choose a reason for hiding this comment

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

@vikasrohit, I've noted about this change #3193 (comment). I just updated it to use the updated service. I'll add the merge logic. I'm writing here because, for some reason, I'm not able to comment on the issue

@vikasrohit
Copy link

I've noted about this change #3193 (comment). I just updated it to use the updated service. I'll add the merge logic. I'm writing here because, for some reason, I'm not able to comment on the issue

@vigneshTheDev I am not able to see the changes

@vigneshTheDev
Copy link
Collaborator Author

@vikasrohit Sorry about the confusion. I haven't added the logic for merging yet.
I was replying to this:

I just noticed that you have made changes for this one as well

I was talking about using the updated scope from the service response instead of updating the status on the fly in the UI.

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.

3 participants