Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

MM-13545 Removal of unused status flags in Schemes requests state object #1007

Merged
merged 4 commits into from
Jan 14, 2020

Conversation

0xVolodya
Copy link
Contributor

@0xVolodya 0xVolodya commented Dec 9, 2019

@sudheerDev sudheerDev self-requested a review December 9, 2019 14:21
@sudheerDev sudheerDev added the 2: Dev Review Requires review by a core commiter label Dec 9, 2019
let data = null;
try {
data = await Client4.deleteScheme(schemeId);
} catch (error) {
forceLogoutIfNecessary(error, dispatch, getState);
dispatch(batchActions([
{type: SchemeTypes.DELETE_SCHEME_FAILURE, error},
logError(error),
Copy link
Contributor

Choose a reason for hiding this comment

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

@nadalfederer hey, Can you add the dispatch of logError back here? We still would like to keep those.

@sudheerDev
Copy link
Contributor

@nadalfederer everything looks good to me other than the one change request.

Copy link
Member

@saturninoabril saturninoabril left a comment

Choose a reason for hiding this comment

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

Tested and looks good, except for the comment from @sudheerDev.

@0xVolodya
Copy link
Contributor Author

@sudheerDev thanks for the review. I fixed it. Off-topic: there is a getSchemeChannels in schemes.ts. I've not found its using in any other files/projects. Isn't it redundant?

@mattermod
Copy link
Contributor

This issue has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @hanzei

@jasonblais
Copy link
Contributor

/update-branch

@mattermod
Copy link
Contributor

Error trying to update the PR.
Please do it manually.

@jasonblais
Copy link
Contributor

@sudheerDev quick reminder to help with reviews after the holidays, thanks! :)

Copy link
Contributor

@sudheerDev sudheerDev left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @nadalfederer 👍

@sudheerDev
Copy link
Contributor

@sudheerDev thanks for the review. I fixed it. Off-topic: there is a getSchemeChannels in schemes.ts. I've not found its using in any other files/projects. Isn't it redundant?

Thanks ok. It is fine to leave. We don't need to remove actions which are not used with this PR.

@nadalfederer Hey, there are few conflicts. Can you resolve them so i can merge it?. Sorry about the delay, Somehow this was out of my review queue until jason requested again.

@sudheerDev sudheerDev added 4: Reviews Complete All reviewers have approved the pull request Awaiting Submitter Action Blocked on the author and removed 2: Dev Review Requires review by a core commiter labels Jan 7, 2020
# Conflicts:
#	src/reducers/requests/index.ts
#	src/types/requests.ts
#	src/types/store.ts
@hanzei hanzei added Awaiting Submitter Action Blocked on the author and removed Awaiting Submitter Action Blocked on the author labels Jan 7, 2020
# Conflicts:
#	src/types/store.ts
@0xVolodya
Copy link
Contributor Author

@sudheerDev hi, I resolved conflicts

@hanzei hanzei removed the Awaiting Submitter Action Blocked on the author label Jan 13, 2020
@hanzei hanzei added this to the v5.20.0 milestone Jan 13, 2020
@sudheerDev sudheerDev merged commit c862447 into mattermost:master Jan 14, 2020
@sudheerDev sudheerDev removed their assignment Jan 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request QA Review Done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removal of unused status flags in Schemes requests state object
7 participants