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

Fix web_client config option #4258

Closed

Conversation

aaronraimist
Copy link
Contributor

@aaronraimist aaronraimist commented Dec 4, 2018

Fixes #1417
Fixes #1883
Fixes #2113
Fixes #1341

I don't think the deleted code in server.py has done anything since ~2015 (since there is no longer a bind_port option) but correct me if I'm wrong.

The other option is to finally merge #2601

Signed-off-by: Aaron Raimist <[email protected]>
Signed-off-by: Aaron Raimist <[email protected]>
@codecov-io
Copy link

Codecov Report

Merging #4258 into develop will increase coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4258      +/-   ##
===========================================
+ Coverage    73.55%   73.56%   +0.01%     
===========================================
  Files          299      299              
  Lines        29806    29798       -8     
  Branches      4872     4871       -1     
===========================================
- Hits         21923    21921       -2     
+ Misses        6456     6447       -9     
- Partials      1427     1430       +3
Impacted Files Coverage Δ
synapse/config/server.py 69.07% <ø> (+5.52%) ⬆️
synapse/app/homeserver.py 56.57% <0%> (-0.38%) ⬇️
synapse/state/v1.py 90.69% <0%> (-1.56%) ⬇️
synapse/handlers/user_directory.py 70.73% <0%> (-0.31%) ⬇️
synapse/handlers/search.py 81.25% <0%> (ø) ⬆️
synapse/handlers/federation.py 61.72% <0%> (ø) ⬆️
synapse/handlers/device.py 81.45% <0%> (+0.8%) ⬆️

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 c033242...c50f87d. Read the comment docs.

@codecov-io
Copy link

codecov-io commented Dec 4, 2018

Codecov Report

Merging #4258 into develop will increase coverage by 0.01%.
The diff coverage is 50%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4258      +/-   ##
===========================================
+ Coverage    73.55%   73.56%   +0.01%     
===========================================
  Files          299      299              
  Lines        29806    29798       -8     
  Branches      4872     4871       -1     
===========================================
- Hits         21923    21921       -2     
+ Misses        6456     6449       -7     
- Partials      1427     1428       +1
Impacted Files Coverage Δ
synapse/app/homeserver.py 56.57% <0%> (-0.38%) ⬇️
synapse/config/server.py 69.07% <100%> (+5.52%) ⬆️
synapse/util/file_consumer.py 80.7% <0%> (-1.76%) ⬇️

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 c033242...2debcec. Read the comment docs.

Vaguely related to this PR but I might as well just fix matrix-org#1341 while I'm here

Signed-off-by: Aaron Raimist <[email protected]>
@richvdh
Copy link
Member

richvdh commented Dec 4, 2018

I don't think the deleted code in server.py has done anything since ~2015 (since there is no longer a bind_port option)

I'm afraid I don't really understand. It looks like you are removing support for the bind_port option: https://github.com/matrix-org/synapse/pull/4258/files#diff-7604431227e17cc581f690570dfacca6L128.

@aaronraimist
Copy link
Contributor Author

@richvdh Isn't this not called bind_port anymore, just port?

186f61a#diff-7604431227e17cc581f690570dfacca6L137

@richvdh
Copy link
Member

richvdh commented Dec 4, 2018

I don't know, but the code you're removing looks like it is looking for bind_port.

@richvdh
Copy link
Member

richvdh commented Dec 11, 2018

superceded by #4290. Thanks for kicking us into sorting this out!

@richvdh richvdh closed this Dec 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants