Skip to content

Large site parsing times may be longer than the Socket URL expiration…#158

Closed
pboucher wants to merge 1 commit into
slackapi:masterfrom
pboucher:master
Closed

Large site parsing times may be longer than the Socket URL expiration…#158
pboucher wants to merge 1 commit into
slackapi:masterfrom
pboucher:master

Conversation

@pboucher

@pboucher pboucher commented Jan 6, 2017 •
edited
Loading

Copy link
Copy Markdown

… time.

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've been mindful about doing atomic commits, adding documentation to my changes, not refactoring too much.
  • I've a descriptive title and added any useful information for the reviewer. Where appropriate, I've attached a screenshot and/or screencast (gif preferably).
  • I've written tests to cover the new code and functionality included in this PR.
  • I've read, agree to, and signed the Contributor License Agreement (CLA).

PR Summary

e.g. New functionality for producing whatsits.

Related Issues

e.g. Fixes #206 and closes #230

Test strategy

e.g. Add tests around whatsit production.

@codecov-io

codecov-io commented Jan 6, 2017

Copy link
Copy Markdown

Current coverage is 62.59% (diff: 0.00%)

Merging #158 into master will not change coverage

@@             master       #158   diff @@
==========================================
  Files             8          8          
  Lines           262        262          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            164        164          
  Misses           98         98          
  Partials          0          0

Powered by Codecov. Last update c990314...4ab834b

@Roach

Roach commented Jan 19, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

@pboucher can you confirm the version of slackclient you're on? This shouldn't be an issue after the latest release.

I don't see a problem merging this, but I'm concerned that it might add a race condition where a developer could connect and immediately attempt to access the user cache before it's populated...

@pboucher

pboucher commented Jan 19, 2017

Copy link
Copy Markdown
Author

The code I had definitely didn't use the SearchDict code which is in 1.0.3 - This PR probably redundant now. I'll test it and maybe we can just close this and not merge it.

@Roach Roach closed this Feb 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

3 participants

@pboucher @codecov-io @Roach