Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unused variables: flake8 --select=F841 #623

Merged
merged 3 commits into from
Oct 7, 2021

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Aug 20, 2021

Fix five of the six flake8 issues related to variables that are assigned to but then not used.

6     F841 local variable 'msg_id' is assigned to but never used

This PR does not fix the following issue where the --unregister_timeout command line option should probably be removed, NOTE: Fixed in second commit.

./rosbridge_server/scripts/rosbridge_websocket.py:194:17: F841 local variable 'unregister_timeout' is assigned to but never used
                unregister_timeout = float(sys.argv[idx])
                ^

@cclauss
Copy link
Contributor Author

cclauss commented Aug 20, 2021

Once #620, #621, #622 and this PR land, we should be able to auto-fix all remaining flake8 issues with:

python3 -m pip install black isort
black .
isort --profile black .

@@ -77,7 +77,7 @@ def fragment(self, message, fragment_size, mid=None):
if message_length <= fragment_size:
return [message]

msg_id = message.get("id", None)
Copy link
Member

Choose a reason for hiding this comment

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

I suggest just deleting this line entirely

Copy link
Contributor Author

@cclauss cclauss Oct 7, 2021

Choose a reason for hiding this comment

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

Done.

@@ -70,7 +70,7 @@ def stopProtocol(self):
rospy.loginfo("Client disconnected. %d clients total.", cls.clients_connected)

def send_message(self, message):
binary = isinstance(message, bson.BSON)
Copy link
Member

Choose a reason for hiding this comment

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

delete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -35,7 +35,7 @@

def calculate_service_response(request):
request_object = json.loads(request) # parse string for service request
args = request_object["args"] # get parameter field (args)
Copy link
Member

@amacneil amacneil Oct 7, 2021

Choose a reason for hiding this comment

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

I think since this is a test this whole block of commented code (line 38-45) seems to be dead, we can delete it

Copy link
Member

Choose a reason for hiding this comment

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

actually, maybe leave it there. I'm not sure if the test is being used. someone has been making PRs to fix these tests so maybe it is helpful for now.

Copy link
Member

@amacneil amacneil left a comment

Choose a reason for hiding this comment

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

thanks!

@amacneil amacneil merged commit 06b4ca2 into RobotWebTools:ros2 Oct 7, 2021
@cclauss cclauss deleted the unused-variables branch October 7, 2021 17:54
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.

None yet

2 participants