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

{Pylint} Fix implicit-str-concat #30363

Merged
merged 5 commits into from
Dec 3, 2024
Merged

{Pylint} Fix implicit-str-concat #30363

merged 5 commits into from
Dec 3, 2024

Conversation

bebound
Copy link
Contributor

@bebound bebound commented Nov 15, 2024

Copy link

azure-client-tools-bot-prd bot commented Nov 15, 2024

️✔️AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.12
️✔️3.9
️✔️ams
️✔️latest
️✔️3.12
️✔️3.9
️✔️apim
️✔️latest
️✔️3.12
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.12
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.12
️✔️3.9
️✔️aro
️✔️latest
️✔️3.12
️✔️3.9
️✔️backup
️✔️latest
️✔️3.12
️✔️3.9
️✔️batch
️✔️latest
️✔️3.12
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.12
️✔️3.9
️✔️billing
️✔️latest
️✔️3.12
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.12
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.12
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.12
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.12
️✔️3.9
️✔️compute_recommender
️✔️latest
️✔️3.12
️✔️3.9
️✔️computefleet
️✔️latest
️✔️3.12
️✔️3.9
️✔️config
️✔️latest
️✔️3.12
️✔️3.9
️✔️configure
️✔️latest
️✔️3.12
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.12
️✔️3.9
️✔️container
️✔️latest
️✔️3.12
️✔️3.9
️✔️containerapp
️✔️latest
️✔️3.12
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.12
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️dls
️✔️latest
️✔️3.12
️✔️3.9
️✔️dms
️✔️latest
️✔️3.12
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.12
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.12
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.12
️✔️3.9
️✔️find
️✔️latest
️✔️3.12
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.12
️✔️3.9
️✔️identity
️✔️latest
️✔️3.12
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️lab
️✔️latest
️✔️3.12
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.12
️✔️3.9
️✔️maps
️✔️latest
️✔️3.12
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.12
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.12
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.12
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.12
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.12
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.12
️✔️3.9
️✔️profile
️✔️latest
️✔️3.12
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.12
️✔️3.9
️✔️redis
️✔️latest
️✔️3.12
️✔️3.9
️✔️relay
️✔️latest
️✔️3.12
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️role
️✔️latest
️✔️3.12
️✔️3.9
️✔️search
️✔️latest
️✔️3.12
️✔️3.9
️✔️security
️✔️latest
️✔️3.12
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.12
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.12
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.12
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.12
️✔️3.9
️✔️sql
️✔️latest
️✔️3.12
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.12
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.12
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️util
️✔️latest
️✔️3.12
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9

Copy link

azure-client-tools-bot-prd bot commented Nov 15, 2024

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Collaborator

yonzhan commented Nov 15, 2024

Thank you for your contribution! We will review the pull request and get back to you soon.

@@ -1136,7 +1136,7 @@ def load_arguments(self, _):
c.argument('subnet_address_prefix', help='The subnet IP address prefix to use when creating a new VNet in CIDR format.')
c.argument('nics', nargs='+', help='Names or IDs of existing NICs to attach to the VM. The first NIC will be designated as primary. If omitted, a new NIC will be created. If an existing NIC is specified, do not specify subnet, VNet, public IP or NSG.')
c.argument('private_ip_address', help='Static private IP address (e.g. 10.0.0.5).')
c.argument('public_ip_address', help='Name of the public IP address when creating one (default) or referencing an existing one. Can also reference an existing public IP by ID or specify "" for None (\'""\' in Azure CLI using PowerShell or --% operator). For Azure CLI using powershell core edition 7.3.4, specify '' or "" (--public-ip-address '' or --public-ip-address "")')
c.argument('public_ip_address', help='Name of the public IP address when creating one (default) or referencing an existing one. Can also reference an existing public IP by ID or specify "" for None (\'""\' in Azure CLI using PowerShell or --% operator). For Azure CLI using powershell core edition 7.3.4, specify or "" (--public-ip-address or --public-ip-address "")')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
c.argument('public_ip_address', help='Name of the public IP address when creating one (default) or referencing an existing one. Can also reference an existing public IP by ID or specify "" for None (\'""\' in Azure CLI using PowerShell or --% operator). For Azure CLI using powershell core edition 7.3.4, specify or "" (--public-ip-address or --public-ip-address "")')
c.argument('public_ip_address', help='Name of the public IP address when creating one (default) or referencing an existing one. Can also reference an existing public IP by ID or specify "" for None (\'""\' in Azure CLI using PowerShell or --% operator). For Azure CLI using powershell core edition 7.3.4, specify or "" (--public-ip-address or --public-ip-address "")')

Copy link
Contributor Author

@bebound bebound Nov 15, 2024

Choose a reason for hiding this comment

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

The ' character is missing. The help message is also not correct. In powershell, the empty string shoule be represented as '""'.

Co-authored-by: Jiashuo Li <4003950+jiasli@users.noreply.github.com>
@wangzelin007
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@wangzelin007
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@bebound bebound marked this pull request as ready for review November 29, 2024 05:39
@@ -438,7 +438,7 @@ def load_arguments(self, _):
c.argument('dapr_enable_api_logging', options_list=['--dapr-enable-api-logging', '--dal'], help="Enable/Disable API logging for the Dapr sidecar.", arg_type=get_three_state_flag(return_label=True))
c.argument('workload_profile_name', help="The name of the workload profile to run the app on.", is_preview=True)
c.argument('cpu', type=float, help="Required CPU in cores from 0.5 to 2.0.", is_preview=True)
c.argument('memory', help="Required momory from 1.0 to 4.0 ending with ""Gi"" e.g. 1.0Gi, ", is_preview=True)
c.argument('memory', help="Required momory from 1.0 to 4.0 ending with Gi e.g. 1.0Gi, ", is_preview=True)
Copy link
Member

Choose a reason for hiding this comment

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

I guess the author wants to quote Gi:

Suggested change
c.argument('memory', help="Required momory from 1.0 to 4.0 ending with Gi e.g. 1.0Gi, ", is_preview=True)
c.argument('memory', help='Required momory from 1.0 to 4.0 ending with "Gi" e.g. 1.0Gi, ', is_preview=True)

@@ -292,7 +292,7 @@ def create_action(action_name, cache_behavior=None, cache_duration=None, header_
action = {
"cache_key_query_string": {
"parameters": {
"type_name" "DeliveryRuleCacheKeyQueryStringBehaviorActionParameters"
"type_name": "DeliveryRuleCacheKeyQueryStringBehaviorActionParameters",
Copy link
Member

Choose a reason for hiding this comment

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

Can't imagine how the code still works while it is as wrong as such.

@@ -42,17 +42,17 @@ def load_arguments(self, _):
'namespace',
completer=get_providers_completion_list,
arg_group='Resource ID',
help='Provider namespace (Ex: ''Microsoft.Provider'').')
help='Provider namespace (Ex: Microsoft.Provider).')
Copy link
Member

Choose a reason for hiding this comment

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

I think the author actually wants to quote Microsoft.Provider, but used the PowerShell syntax:

> echo "a""b"
a"b
> echo 'a''b'
a'b
Suggested change
help='Provider namespace (Ex: Microsoft.Provider).')
help="Provider namespace (Ex: 'Microsoft.Provider').")

Copy link
Member

Choose a reason for hiding this comment

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

Should the following three help messages be kept consistent? Personally, I think when parentheses are used, having or not having double quotes is fine.

Copy link
Member

@jiasli jiasli left a comment

Choose a reason for hiding this comment

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

Let's not assume what the author wants to quote. Just merge this PR and let the author change it in the future.

@bebound bebound merged commit 978b94c into Azure:dev Dec 3, 2024
53 checks passed
@bebound bebound deleted the fix-str branch December 3, 2024 09:05
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.

8 participants