OpenMined / KotlinSyft

The official Syft worker for secure on-device machine learning

https://www.openmined.org

Apache License 2.0

85 stars 28 forks source link

revise readme and cleartextTraffic config #240

Closed pengyuan-zhou closed 4 years ago

pengyuan-zhou commented 4 years ago

For readme: the url on login needs the port as well according to my test. For cleartextTraffic config, please refer to the discussion here

codecov[bot] commented 4 years ago

Codecov Report

Merging #240 into dev will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev     #240   +/-   ##
=======================================
  Coverage   59.97%   59.97%           
=======================================
  Files          44       44           
  Lines        1464     1464           
  Branches      193      193           
=======================================
  Hits          878      878           
  Misses        501      501           
  Partials       85       85

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 9bc8e1c...c61f80b. Read the comment docs.

vkkhare commented 4 years ago

does this PR still needs merging? @pengyuan-zhou

pengyuan-zhou commented 4 years ago

does this PR still needs merging? @pengyuan-zhou

I can give another look on how to auto-import the url during the weekend, of course we can also just mention it somewhere like in README or tutorial.

pengyuan-zhou commented 4 years ago

does this PR still needs merging? @pengyuan-zhou

Didn't manage the autoconfig url. Added 10.0.2.2 as a basic permitted url and the instruction to reconfig it in README.

vkkhare commented 4 years ago

No we don't want the users to configure the file for urls. It was like that earlier and made things difficult for users to start. Better leave that as an attribute of the activity so that it's present for all the urls. This is only meant for demo anyway.

For this PR better have only the readme changes

pengyuan-zhou commented 4 years ago

No we don't want the users to configure the file for urls. It was like that earlier and made things difficult for users to start. Better leave that as an attribute of the activity so that it's present for all the urls. This is only meant for demo anyway.

For this PR better have only the readme changes

Alright, now the commits are rebased.