Bitmessage/PyBitmessage-2024-11-28
Archived
2
0
Fork 0
Code Issues 168 Pull Requests 53 Actions Packages Projects Releases 5 Wiki Activity

Changes based on style and lint checks. Some empty docstrings remain … #1368

Closed
coffeedogs wants to merge 1 commits from final_code_quality_10 into v0.6
pull from: final_code_quality_10
merge into: Bitmessage:v0.6
Conversation 0 Commits 1 Files Changed 6 +259 -211
coffeedogs commented 2018-10-10 13:26:17 +02:00 (Migrated from github.com)
Copy Link

…to be completed in the future. (final_code_quality_10)

Repository contributions to the PyBitmessage project

  • You can get paid for merged commits if you register at Tip4Commit

Code

  • Try to refer to github issue tracker or other permanent sources of discussion about the issue.
  • It is clear from the diff what you have done, it may be less clear why you have done it so explain why this change is necessary rather than what it does

Documentation

  • If there has been a change to the code, there's a good possibility there should be a corresponding change to the documentation
  • If you can't run fab build_docs successfully, ask for someone to run it against your branch

Tests

  • If there has been a change to the code, there's a good possibility there should be a corresponding change to the tests
  • If you can't run fab tests successfully, ask for someone to run it against your branch

Translations

  • For helping with translations, please use Transifex.
  • There is no need to submit pull requests for translations.
  • For translating technical terms it is recommended to consult the Microsoft Language Portal.

Gitiquette

  • Make the pull request against the "v0.6" branch
  • PGP-sign the commits included in the pull request
  • Use references to tickets, e.g. addresses #123 or fixes #234 in your commit messages
  • Try to use a good editor that removes trailing whitespace, highlights potential python issues and uses unix line endings
  • If for some reason you don't want to use github, you can submit the patch using Bitmessage to the "bitmessage" chan, or to one of the developers.
…to be completed in the future. (final_code_quality_10) ## Repository contributions to the PyBitmessage project - You can get paid for merged commits if you register at [Tip4Commit](https://tip4commit.com/github/Bitmessage/PyBitmessage) ### Code - Try to refer to github issue tracker or other permanent sources of discussion about the issue. - It is clear from the diff *what* you have done, it may be less clear *why* you have done it so explain why this change is necessary rather than what it does ### Documentation - If there has been a change to the code, there's a good possibility there should be a corresponding change to the documentation - If you can't run `fab build_docs` successfully, ask for someone to run it against your branch ### Tests - If there has been a change to the code, there's a good possibility there should be a corresponding change to the tests - If you can't run `fab tests` successfully, ask for someone to run it against your branch ## Translations - For helping with translations, please use [Transifex](https://www.transifex.com/bitmessage-project/pybitmessage/). - There is no need to submit pull requests for translations. - For translating technical terms it is recommended to consult the [Microsoft Language Portal](https://www.microsoft.com/Language/en-US/Default.aspx). ### Gitiquette - Make the pull request against the ["v0.6" branch](https://github.com/Bitmessage/PyBitmessage/tree/v0.6) - PGP-sign the commits included in the pull request - Use references to tickets, e.g. `addresses #123` or `fixes #234` in your commit messages - Try to use a good editor that removes trailing whitespace, highlights potential python issues and uses unix line endings - If for some reason you don't want to use github, you can submit the patch using Bitmessage to the "bitmessage" chan, or to one of the developers.
PeterSurda (Migrated from github.com) reviewed 2018-10-19 09:55:23 +02:00
src/bitmessageqt/migrationwizard.py
PeterSurda (Migrated from github.com) commented 2018-10-19 09:55:23 +02:00

Unused, unfinished and obsolete. Remove whole file.

Unused, unfinished and obsolete. Remove whole file.
PeterSurda (Migrated from github.com) reviewed 2018-10-19 09:56:20 +02:00
src/bitmessageqt/safehtmlparser.py
PeterSurda (Migrated from github.com) commented 2018-10-19 09:56:20 +02:00 
    """HTML parser with sanitisation"""
```suggestion """HTML parser with sanitisation""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:00:22 +02:00
src/bitmessageqt/safehtmlparser.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:00:22 +02:00 
        """Perform substring replacement before regex replacements"""
```suggestion """Perform substring replacement before regex replacements""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:00:56 +02:00
src/bitmessageqt/safehtmlparser.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:00:56 +02:00 
        """Perform substring replacement after regex replacements"""
```suggestion """Perform substring replacement after regex replacements""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:03:08 +02:00
src/bitmessageqt/safehtmlparser.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:03:08 +02:00 
        """Reset runtime variables specific to this class"""
```suggestion """Reset runtime variables specific to this class""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:03:57 +02:00
src/bitmessageqt/safehtmlparser.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:03:57 +02:00 
        """Add tag if it passes sanitisation"""
```suggestion """Add tag if it passes sanitisation""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:04:54 +02:00
src/bitmessageqt/safehtmlparser.py
@ -101,3 +125,4 @@
""""""
try:
data = unicode(data, 'utf-8')
except UnicodeDecodeError:
PeterSurda (Migrated from github.com) commented 2018-10-19 10:04:54 +02:00 
        """Detect if string contains HTML tags"""
```suggestion """Detect if string contains HTML tags""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:06:02 +02:00
src/bitmessageqt/support.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:06:02 +02:00 
    """Check if the support bitmessage address is in addressbook, and add it if necessary. Also delete the old one which is considered compromised."""
```suggestion """Check if the support bitmessage address is in addressbook, and add it if necessary. Also delete the old one which is considered compromised.""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:07:08 +02:00
src/bitmessageqt/support.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:07:08 +02:00 
    """Check if a normal (non-chan, non-subscription) address exists"""
```suggestion """Check if a normal (non-chan, non-subscription) address exists""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:07:32 +02:00
src/bitmessageqt/support.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:07:32 +02:00 
    """Create a normal address (for sending the support request) in case it doesn't exist already"""
```suggestion """Create a normal address (for sending the support request) in case it doesn't exist already""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:08:39 +02:00
src/bitmessageqt/support.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:08:39 +02:00 
    """Create a support message (pre-fill from system info)"""
```suggestion """Create a support message (pre-fill from system info)""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:10:10 +02:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:10:10 +02:00 
    """Pool of all existing connections"""
```suggestion """Pool of all existing connections""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:10:33 +02:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:10:32 +02:00 
        """Connect to a bitmessage stream"""
```suggestion """Connect to a bitmessage stream""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:11:43 +02:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:11:43 +02:00 
        """Return an (existing) connection object based on a `Peer` object (IP and port)"""
```suggestion """Return an (existing) connection object based on a `Peer` object (IP and port)""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:12:05 +02:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:12:05 +02:00 
        """Check if we're already connected to this peer"""
```suggestion """Check if we're already connected to this peer""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:12:49 +02:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:12:49 +02:00 
        """Add a connection object to our internal dict"""
```suggestion """Add a connection object to our internal dict""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:13:07 +02:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:13:07 +02:00 
        """Remove a connection from our internal dict"""
```suggestion """Remove a connection from our internal dict""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:13:56 +02:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:13:55 +02:00 
        """What IP are we supposed to be listening on?"""
```suggestion """What IP are we supposed to be listening on?""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:14:39 +02:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:14:39 +02:00 
        """Open a listening socket and start accepting connections on it"""
```suggestion """Open a listening socket and start accepting connections on it""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:16:45 +02:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:16:44 +02:00 
        """Open an UDP socket. Depending on settings, it can either only accept incoming UDP packets, or also be able to send them."""
```suggestion """Open an UDP socket. Depending on settings, it can either only accept incoming UDP packets, or also be able to send them.""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:18:03 +02:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:18:03 +02:00

This may have changed in the meantime, please rebase so that I can see.

This may have changed in the meantime, please rebase so that I can see.
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:22:15 +02:00
src/network/connectionpool.py
@ -242,4 +253,3 @@
reaper = []
for i in self.inboundConnections.values() + self.outboundConnections.values():
minTx = time.time() - 20
if i.fullyEstablished:
PeterSurda (Migrated from github.com) commented 2018-10-19 10:22:15 +02:00

Since you refactored the cycle, you don't need reaper anymore, you can directly call self.removeConnection(i). The reason why it's necessary to do it in two cycles is that you can't remove items from a dict while iterating it.

Since you refactored the cycle, you don't need reaper anymore, you can directly call `self.removeConnection(i)`. The reason why it's necessary to do it in two cycles is that you can't remove items from a dict while iterating it.
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:23:36 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:23:36 +02:00 
    ""Base proxy exception class"""
```suggestion ""Base proxy exception class""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:24:19 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:24:19 +02:00 
    """General proxy error class (not specfic to an implementation)"""
```suggestion """General proxy error class (not specfic to an implementation)""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:24:46 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:24:46 +02:00 
    """Base proxy class for AdvancedDispatcher"""
```suggestion """Base proxy class for AdvancedDispatcher""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:27:02 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:27:01 +02:00 
        """Return proxy IP and port"""
```suggestion """Return proxy IP and port""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:27:21 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:27:21 +02:00 
        """Set proxy IP and port"""
```suggestion """Set proxy IP and port""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:28:29 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:28:29 +02:00 
        """Return proxy authentication settings"""
```suggestion """Return proxy authentication settings""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:29:51 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:29:51 +02:00 
        """Set proxy authentication (username and password)"""
```suggestion """Set proxy authentication (username and password)""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:30:35 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:30:35 +02:00 
        """Return separate proxy IP and port for use only with onion addresses. Untested."""
```suggestion """Return separate proxy IP and port for use only with onion addresses. Untested.""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:31:12 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:31:12 +02:00 
        """Return proxy authentication for onion hosts only. Untested."""
```suggestion """Return proxy authentication for onion hosts only. Untested.""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:33:58 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:33:58 +02:00 
        """Set proxy authentication for onion hosts only. Untested"""
```suggestion """Set proxy authentication for onion hosts only. Untested""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:37:20 +02:00
src/network/proxy.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:37:20 +02:00 
        """Handle connection event (to the proxy)"""
```suggestion """Handle connection event (to the proxy)""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:38:24 +02:00
src/storage/filesystem.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:38:24 +02:00 
    """Module for using filesystem (directory with files) for inventory storage"""
```suggestion """Module for using filesystem (directory with files) for inventory storage""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:47:30 +02:00
src/storage/filesystem.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:47:30 +02:00 
        """Remove object from inventory"""
```suggestion """Remove object from inventory""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:49:02 +02:00
src/storage/filesystem.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:49:02 +02:00 
        """Return list of streams"""
```suggestion """Return list of streams""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:59:37 +02:00
src/storage/filesystem.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:59:37 +02:00 
        """Return inventory vectors (hashes) from a directory"""
```suggestion """Return inventory vectors (hashes) from a directory""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 10:59:53 +02:00
src/storage/filesystem.py
PeterSurda (Migrated from github.com) commented 2018-10-19 10:59:53 +02:00 
        """Get object data"""
```suggestion """Get object data""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 11:00:14 +02:00
src/storage/filesystem.py
PeterSurda (Migrated from github.com) commented 2018-10-19 11:00:14 +02:00 
        """Get object metadata"""
```suggestion """Get object metadata""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 11:01:27 +02:00
src/storage/filesystem.py
@ -144,3 +172,4 @@
try:
if item.payload is None:
item.payload = self.getData(hashId)
except IOError:
PeterSurda (Migrated from github.com) commented 2018-10-19 11:01:26 +02:00 
        """Return inventory vectors (hashes) for a stream"""
```suggestion """Return inventory vectors (hashes) for a stream""" ```
PeterSurda (Migrated from github.com) reviewed 2018-10-19 11:02:51 +02:00
src/storage/filesystem.py
@ -162,12 +193,14 @@ class FilesystemInventory(InventoryStorage):
return []
PeterSurda (Migrated from github.com) commented 2018-10-19 11:02:51 +02:00

You can do a for _, streamDict instead of # pylint: disable=unused-variable.

You can do a `for _, streamDict` instead of `# pylint: disable=unused-variable`.
PeterSurda (Migrated from github.com) reviewed 2018-10-19 11:04:00 +02:00
PeterSurda (Migrated from github.com) left a comment

Update docstrings etc.

Update docstrings etc.
coffeedogs (Migrated from github.com) reviewed 2018-10-19 15:46:05 +02:00
src/bitmessageqt/migrationwizard.py
coffeedogs (Migrated from github.com) commented 2018-10-19 15:46:05 +02:00

+1 for ruthlessness :)

+1 for ruthlessness :)
coffeedogs (Migrated from github.com) reviewed 2018-10-19 16:30:18 +02:00
src/network/connectionpool.py
coffeedogs (Migrated from github.com) commented 2018-10-19 16:30:18 +02:00

Rebased.

Rebased.
g1itch (Migrated from github.com) reviewed 2018-10-19 16:32:44 +02:00
src/network/connectionpool.py
g1itch (Migrated from github.com) commented 2018-10-19 16:32:44 +02:00

No, helper_bootstrap.dns() is above. That's enough.

No, `helper_bootstrap.dns()` is above. That's enough.
PeterSurda (Migrated from github.com) reviewed 2018-11-02 06:54:17 +01:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-11-02 06:54:17 +01:00

I'll accept it as it is now, it isn't a code quality issue, it can be changed in a separate PR.

I'll accept it as it is now, it isn't a code quality issue, it can be changed in a separate PR.
PeterSurda (Migrated from github.com) approved these changes 2018-11-02 06:58:41 +01:00
g1itch (Migrated from github.com) reviewed 2018-11-05 18:43:52 +01:00
src/network/connectionpool.py
g1itch (Migrated from github.com) commented 2018-11-05 18:43:52 +01:00

@PeterSurda any PR makes no sense if changes reverted soon after merge.
I spent a week to find a proper way to check knownnodes actuality. If someone thinks I did it wrong, he should create a separate issue, not revert my changes in unrelated PR.

@PeterSurda any PR makes no sense if changes reverted soon after merge. I spent a week to find a proper way to check knownnodes actuality. If someone thinks I did it wrong, he should create a separate issue, not revert my changes in unrelated PR.
PeterSurda (Migrated from github.com) reviewed 2018-11-05 19:01:05 +01:00
src/network/connectionpool.py
PeterSurda (Migrated from github.com) commented 2018-11-05 19:01:05 +01:00

I don't have time to debug so let's go with what @g1itch said as he tested it. Please revert.

I don't have time to debug so let's go with what @g1itch said as he tested it. Please revert.
PeterSurda (Migrated from github.com) requested changes 2018-11-05 19:02:40 +01:00
PeterSurda (Migrated from github.com) left a comment

Please revert the changes as @g1itch said.

Please revert the changes as @g1itch said.
coffeedogs (Migrated from github.com) reviewed 2018-11-12 16:04:16 +01:00
src/network/connectionpool.py
coffeedogs (Migrated from github.com) commented 2018-11-12 16:04:16 +01:00

So, I removed line 160 which I guess must have been added by git during the rebase. All other active lines are as they were. If there's something else I'm supposed to have reverted, let me know.

So, I removed line 160 which I guess must have been added by git during the rebase. All other active lines are as they were. If there's something else I'm supposed to have reverted, let me know.
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
PeterSurda
Labels
Clear labels   
bug

Something isn't working as it's supposed to

  
build

Related to building or build system

  
dependencies

Pull requests that update a dependency file

  
developers

The issue is relevant mainly for developers rather than users

  
documentation

   
duplicate

   
enhancement

New feature

  
formatting

Code or UI text formatting

  
invalid

   
legal

   
mobile

Frontend for mobile devices (kivy)

  
obsolete

   
packaging

   
performance

   
protocol

Protocol change

  
question

   
refactoring

   
regression

Some issue, fixed in the past, appears again

  
security

   
test

   
translation

   
usability

   
wontfix
No Label
bug
build
dependencies
developers
documentation
duplicate
enhancement
formatting
invalid
legal
mobile
obsolete
packaging
performance
protocol
question
refactoring
regression
security
test
translation
usability
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Bitmessage/PyBitmessage-2024-11-28#1368
No description provided.
Delete Branch "final_code_quality_10"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?