Skip to content

Conversation

@madeye
Copy link
Contributor

@madeye madeye commented May 7, 2020

Details in #238

@madeye madeye requested a review from Mygod May 7, 2020 07:17
Copy link
Contributor

@Mygod Mygod left a comment

Choose a reason for hiding this comment

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

Looks good to me. Although (completely unrelated to changes currently) maybe we should remove cache entries when adding cache, for example:

  1. 1.1.1.1 is not proxied by default;
  2. domain.com is resolved to 1.1.1.1 and gets proxied, and this gets added to the cache;
  3. anotherdomain.com is also resolved to 1.1.1.1 and gets bypassed -- we should still update the cache.

@madeye
Copy link
Contributor Author

madeye commented May 7, 2020

Although (completely unrelated to changes currently) maybe we should remove cache entries when adding cache

Let's make this a future improvement, although this use case doesn't make sense to me...

@madeye madeye requested a review from zonyitoo May 7, 2020 07:35
@Mygod
Copy link
Contributor

Mygod commented May 7, 2020

It's an edge case rather than a use case to be honest XD (it offers better consistency rather than utility).

@zonyitoo
Copy link
Collaborator

zonyitoo commented May 7, 2020

Actually I don't know why Android needs so many complicated modification :P

@zonyitoo zonyitoo merged commit 0b84a85 into shadowsocks:master May 7, 2020
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