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

Replacing blacklist with blocklist #5952

Merged
merged 1 commit into from
Jul 23, 2020
Merged

Replacing blacklist with blocklist #5952

merged 1 commit into from
Jul 23, 2020

Conversation

jespino
Copy link
Member

@jespino jespino commented Jul 21, 2020 •
edited by lindy65
Loading

Summary

Replacing blacklist with blocklist

Jira ticket: https://mattermost.atlassian.net/browse/MM-27193

@jespino jespino added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Jul 21, 2020
@jespino jespino removed the 1: PM Review Requires review by a product manager label Jul 21, 2020
@hanzei
Copy link
Contributor

hanzei commented Jul 21, 2020

Drive by comment: Is the decision to use certain terms like "Blocklist" worth documenting somewhere e.g. in https://developers.mattermost.com/contribute/getting-started/?

@jasonblais jasonblais mentioned this pull request Jul 22, 2020
Merged
@jespino
Copy link
Member Author

jespino commented Jul 22, 2020

@hanzei I think would be great to have some documentation around it, but we should decide what is the things that we want to try avoid in our code, blacklist was an easy one, but there are other. @jasonblais What do you think? is better to add something now about blacklist and grow from there, or is better to wait until have something more organized and write a more clear policy?

@jasonblais
Copy link
Contributor

I would wait until we've formalized a policy

sudheerDev
sudheerDev approved these changes Jul 22, 2020
View reviewed changes
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

@sudheerDev sudheerDev removed the 2: Dev Review Requires review by a core commiter label Jul 22, 2020
@jespino jespino requested a review from lindy65 July 22, 2020 13:42
@lindy65 lindy65 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jul 23, 2020
lindy65
lindy65 approved these changes Jul 23, 2020
View reviewed changes
Copy link
Contributor

@lindy65 lindy65 left a comment

Choose a reason for hiding this comment

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

Thanks @jespino - I smoke tested around emoji / custom emoji and did not find any issues 👍

@lindy65 lindy65 added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Jul 23, 2020
@mm-cloud-bot
Copy link

Test server destroyed

@hanzei
Copy link
Contributor

hanzei commented Jul 23, 2020

I'm fine with waiting for a well defined policy as long it get added eventually.

@jespino jespino merged commit 1914a6a into mattermost:master Jul 23, 2020
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jul 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Willyfrog Willyfrog Willyfrog approved these changes

@sudheerDev sudheerDev sudheerDev approved these changes

@lindy65 lindy65 lindy65 approved these changes

@jasonblais jasonblais jasonblais approved these changes

Assignees

@jespino jespino

Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation
Projects
None yet
Milestone
No milestone
8 participants
@jespino @hanzei @jasonblais @mm-cloud-bot @Willyfrog @sudheerDev @lindy65 @amyblais