Skip to content

Network Quality fixes 1 - #1554

Merged
PeterSurda merged 2 commits into
Bitmessage:v0.6from
lakshyacis:fix#11
Jan 13, 2020
Merged

PeterSurda merged 2 commits into
Bitmessage:v0.6from
lakshyacis:fix#11

Conversation

@lakshyacis

@lakshyacis lakshyacis commented Nov 12, 2019 •

Copy link
Copy Markdown
Contributor

Changes based on flake8 and pylint.
Reduce line length| Suppressed warnings| Added docstrings.

@lakshyacis
lakshyacis force-pushed the fix#11 branch 4 times, most recently from 7d4967d to 8984dcd Compare November 18, 2019 13:44
Comment thread src/network/__init__.py Outdated
Comment thread src/network/addrthread.py Outdated
Comment thread src/network/addrthread.py Outdated
Comment thread src/network/connectionchooser.py Outdated
Comment thread src/network/connectionchooser.py Outdated
Comment thread src/network/networkthread.py Outdated
Comment thread src/network/receivequeuethread.py Outdated

@PeterSurda PeterSurda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please make the requested changes.

@lakshyacis
lakshyacis force-pushed the fix#11 branch 3 times, most recently from ca671bd to 2216c60 Compare November 25, 2019 13:41
Comment thread src/network/bmproto.py Outdated
@PeterSurda

Copy link
Copy Markdown
Member

please remove bitmessagecureses/__init__py from the PR

@PeterSurda
PeterSurda self-requested a review November 25, 2019 14:30

@PeterSurda PeterSurda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please make the requested changes

Comment thread src/network/addrthread.py Outdated
Comment thread src/network/addrthread.py Outdated
Comment thread src/network/receivequeuethread.py Outdated
Comment thread src/network/addrthread.py
Comment thread src/network/networkthread.py
Comment thread src/network/bmproto.py Outdated
@g1itch

g1itch commented Nov 25, 2019 •

Copy link
Copy Markdown
Collaborator

So inconsistent. I don't understand why do you

  • edit plugins again;
  • document run() only in AddrThread;
  • edit unrelated modules in single PR.

@lakshyacis

Copy link
Copy Markdown
Contributor Author

So inconsistent. I don't understand why do you

  • edit plugins again;
  • document run() only in AddrThread;
  • edit unrelated modules in single PR.

Actually, all the code quality stuff is done. So, I'm just looking for the files that needed fixes. That's why I edited plugins again and this PR contains unrelated modules. But I will remove unrelated modules from the PR and make a new PR for them.
Thank you

@lakshyacis lakshyacis changed the title Code quality fixes Code quality fixes(13) Nov 26, 2019
Comment thread src/network/assemble.py Outdated
@@ -50,16 +45,17 @@
many of the difficult problems for you, making the task of building
sophisticated high-performance network servers and clients a snap.
"""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do you want to include license into the doc?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't know about this, that's why I didn't remove. Should I remove that?

@lakshyacis
lakshyacis force-pushed the fix#11 branch 2 times, most recently from 497eb09 to 6df4150 Compare December 26, 2019 07:24
@lakshyacis
lakshyacis force-pushed the fix#11 branch 15 times, most recently from f178ae4 to 8eb4535 Compare January 8, 2020 08:16
@lakshyacis
lakshyacis requested a review from PeterSurda January 8, 2020 13:17

@PeterSurda PeterSurda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please change the two docstrings I noted, otherwise ok.

Comment thread src/network/bmproto.py Outdated
Comment thread src/network/bmproto.py Outdated

@PeterSurda PeterSurda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add the license you removed from src/network/asyncore_pollchoose.py into LICENSE file in the project root, after the other file-specific licenses. Otherwise ok.

Comment thread LICENSE Outdated

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDER ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

========================================================================================================

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you should indicate which file this refers to, like on lines 23 and 37

@PeterSurda
PeterSurda merged commit 22e2263 into Bitmessage:v0.6 Jan 13, 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