Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panic if channel is meeting is not found #59

Merged
merged 1 commit into from
Sep 22, 2020
Merged

Fix panic if channel is meeting is not found #59

merged 1 commit into from
Sep 22, 2020

Conversation

hanzei
Copy link
Contributor

@hanzei hanzei commented Sep 22, 2020

Summary

Fix a panic if the meeting for autocomplete and add a log message.

Ticket Link

https://community-daily.mattermost.com/core/pl/g5c8dwh5ufrtpprtontzeecnuc

@hanzei hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Sep 22, 2020
@hanzei hanzei added this to the v0.3.0 milestone Sep 22, 2020
@hanzei hanzei modified the milestones: v0.3.0, v0.2.1 Sep 22, 2020
@codecov
Copy link

codecov bot commented Sep 22, 2020

Codecov Report

Merging #59 into master will decrease coverage by 0.16%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
- Coverage   26.04%   25.87%   -0.17%     
==========================================
  Files           6        6              
  Lines         311      313       +2     
==========================================
  Hits           81       81              
- Misses        213      215       +2     
  Partials       17       17              
Impacted Files Coverage Δ
server/plugin.go 22.58% <0.00%> (-0.50%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd0e9dd...9e0462c. Read the comment docs.

Copy link
Contributor

@marianunez marianunez left a comment

Choose a reason for hiding this comment

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

Thanks @hanzei!

@DHaussermann
Copy link

@hanzei any thoughts on testing this? Would I need a way to simulate such an error?

@hanzei
Copy link
Contributor Author

hanzei commented Sep 22, 2020

@DHaussermann Not really. I don't know what causes the crash, which is why I added the log statement.

@hanzei hanzei mentioned this pull request Sep 22, 2020
4 tasks
@hanzei hanzei removed the 2: Dev Review Requires review by a core committer label Sep 22, 2020
@hanzei
Copy link
Contributor Author

hanzei commented Sep 22, 2020

@DHaussermann Would you be fine skipping QA review?

@DHaussermann
Copy link

@hanzei, makes sense to me. As it is there is no functional testing I can do.
LGTM!

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Sep 22, 2020
@hanzei hanzei merged commit 4d85444 into master Sep 22, 2020
@hanzei hanzei deleted the fix_panic branch September 22, 2020 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants