8000 8000
Skip to content

lxd: Remove lint exception for defer rule.#14649

Merged
tomponline merged 1 commit intocanonical:mainfrom
markylaing:defer-internal-import-from-backup
Dec 13, 2024
Merged

lxd: Remove lint exception for defer rule.#14649
tomponline merged 1 commit intocanonical:mainfrom
markylaing:defer-internal-import-from-backup

Conversation

@markylaing
Copy link
Copy Markdown
Contributor

The defer rule was removed from our golangci-lint configuration in 0b3a094. With this change, the defer in this function (which is called in a for loop) no longer needs to be ignored by the linter.

The defer rule was originally removed because refactoring existing code to stop calling defer within loops was resulting in subtle bugs that were hard to reason about.

We have extensive tests for LXD, so we should:

  • Avoid calling defer inside a for loop, and watch for these in pull requests.
  • Focus on improving code quality from a TICS perspective first.

Closes #12854

The defer rule was removed from our golangci-lint configuration in
0b3a094. With this change, the
defer in this function (which is called in a for loop) no longer needs
to be ignored by the linter.

The defer rule was originally removed because refactoring existing code
to stop calling defer within loops was resulting in subtle bugs that
were hard to reason about.

We have extensive tests for LXD, so we should:
- Avoid calling defer inside a for loop, and watch for these in pull
  requests.
- Focus on improving code quality from a TICS perspective first.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing markylaing self-assigned this Dec 13, 2024
Copy link
Copy Markdown
Member
@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Are there any other ones like this we should remove?

@tomponline tomponline merged commit 2f4b5ff into canonical:main Dec 13, 2024
@markylaing
Copy link
Copy Markdown
Contributor Author

Are there any other ones like this we should remove?

I'll do a quick grep and have a look :)

tomponline added a commit that referenced this pull request Dec 17, 2024
Some `//nolint` comments have become defunct after changes to the linter
configuration. This PR removes them!

Follow up to #14649
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.

Investigate linter error in internalImportFromBackup

2 participants

0