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 pylint for container.py #2

Conversation

stevendongatmsft
Copy link
Collaborator

@stevendongatmsft stevendongatmsft commented Feb 2, 2023

pylint: disable=too-many-instance-attributes is added because I don't think our class design has any issues. We don't want to separate things that are supposed to be together into different classes just to accommodate pylint. Eventually imo pylint is just recommendations of what it thinks could be improved.

we can discuss pylint: disable=line-too-long. Imo it's the best way to deal with this pylint error.

config.POLICY_FIELD_CONTAINERS_ELEMENTS_ENVS_RULE:
f"{rule[config.ACI_FIELD_CONTAINERS_ENVS_NAME]}="
# pylint: disable=line-too-long
config.POLICY_FIELD_CONTAINERS_ELEMENTS_ENVS_RULE: f"{rule[config.ACI_FIELD_CONTAINERS_ENVS_NAME]}="
Copy link
Owner

Choose a reason for hiding this comment

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

does having a line break here not work? I think that would be nicer than disabling pylint

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have this auto format and every time when I do line break it changes back to this.

Copy link
Owner

@SethHollandsworth SethHollandsworth left a comment

Choose a reason for hiding this comment

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

a few things to look at, good stuff

@stevendongatmsft stevendongatmsft self-assigned this Feb 2, 2023
Copy link
Owner

@SethHollandsworth SethHollandsworth left a comment

Choose a reason for hiding this comment

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

lgtm

@SethHollandsworth SethHollandsworth merged this pull request into SethHollandsworth:confcom-onboarding Feb 2, 2023
SethHollandsworth pushed a commit that referenced this pull request Aug 14, 2024
* fix: --title --kind --custom-property parameters (#1)

* feat: support yaml file in register command (#3)

* fix: workspace parameter should not be required (#2)

* style: fix lint issues (#6)

* chore: update codeowner for apic-extension (#5)

* feat: revert changes to min cli version (#4)

* feat: resolve comments to CLI experience (#10)

* feat: mark "apic api register" command as preview (#13)

* feat: remove --terms-of-service parameter (#12)

* feat: remove --workspace-name parameter (#11)

* feat: support python 3.8 and 3.9 (#14)

* feat: fix command descriptions (#17)

* tests: add test cases for 'apic service' commands (#16)

* feat: Use 03-01 spec from azure-rest-api-specs repo to regenerate CLI (#19)

* fix: CLI errors (#20)

* fix: import error when run CLI command

* fix: no workspace name error when run import specification command

* feat: mark some parameters as required (#21)

* feat: mark --assignments parameter required for metadata commands

* doc: update sample commands

* feat: mark --source-resource-ids parameter as required for import-from-apim command

* doc: update description for import-from-apim command

* feat: bump version to 1.0.0b5 and update changelog (#18)

* build: fix ci (#22)

* style: fix style warnings

* test:fix test cases

* fix: service update command failure (#28)

* doc: fix command samples (#30)

* doc: update changelog per feedback (#29)

* doc: remove extra spaces in register command help message (#34)

* doc: update api definition and metadata command samples (#36)
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.

2 participants