Skip to content

Conversation

@jiasli
Copy link
Member

@jiasli jiasli commented May 7, 2021

Description

For unknown reason, Pylint 2.3.0 fails to find linter errors in _github_oauth.py added by #17826.

After bumping Pylint 2.8.0 (#17861), those errors are revealed.

https://dev.azure.com/azure-sdk/public/_build/results?buildId=880573&view=logs&j=36dd4138-4d53-5e46-00d9-e5cd9744be05&t=1cf3879c-0a42-5ccd-7693-7a4781d739d8&l=107

src/azure-cli/azure/cli/command_modules/appservice/_github_oauth.py:20:28: W0613: Unused argument 'cmd' (unused-argument)

@jiasli jiasli requested review from Juliehzl and qwordy as code owners May 7, 2021 07:35
@yonzhan
Copy link
Collaborator

yonzhan commented May 7, 2021

AppService

@yonzhan yonzhan added this to the S187 milestone May 7, 2021


def get_github_access_token(cmd, scope_list=None):
def get_github_access_token(cmd, scope_list=None): # pylint: disable=unused-argument
Copy link
Member

Choose a reason for hiding this comment

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

How about remove cmd parameter directly? As far as I know, custom function is allowed to not contain cmd according to

def default_command_handler(command_args):
from azure.cli.core.util import get_arg_list, augment_no_wait_handler_args
from azure.cli.core.commands.client_factory import resolve_client_arg_name
op = handler or self.get_op_handler(operation, operation_group=kwargs.get('operation_group'))
op_args = get_arg_list(op)
cmd = command_args.get('cmd') if 'cmd' in op_args else command_args.pop('cmd')
client = client_factory(cmd.cli_ctx, command_args) if client_factory else None
supports_no_wait = kwargs.get('supports_no_wait', None)
if supports_no_wait:
no_wait_enabled = command_args.pop('no_wait', False)
augment_no_wait_handler_args(no_wait_enabled, op, command_args)
if client:
client_arg_name = resolve_client_arg_name(operation, kwargs)
if client_arg_name in op_args:
command_args[client_arg_name] = client
return op(**command_args)

L813 will process smartly

Copy link
Member Author

@jiasli jiasli May 7, 2021

Choose a reason for hiding this comment

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

This get_github_access_token is not used anywhere. Leave it to the author @calvinsID to further decide.

Copy link
Member

@evelyn-ys evelyn-ys left a comment

Choose a reason for hiding this comment

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

LGTM in general

@jiasli jiasli merged commit c06c982 into Azure:dev May 7, 2021
@jiasli jiasli deleted the linter branch May 7, 2021 08:18
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.

4 participants