Skip to content

fix: octalLiteral from go-critic#8811

Merged
knqyf263 merged 1 commit intoaquasecurity:mainfrom
mmorel-35:go-critic/octalLiteral
May 5, 2025
Merged

fix: octalLiteral from go-critic#8811
knqyf263 merged 1 commit intoaquasecurity:mainfrom
mmorel-35:go-critic/octalLiteral

Conversation

@mmorel-35
Copy link
Copy Markdown
Contributor

@mmorel-35 mmorel-35 commented May 2, 2025

Description

Fixes octalLiteral rule from go-critic

Remove this section if you don't have related PRs.

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@mmorel-35 mmorel-35 force-pushed the go-critic/octalLiteral branch 5 times, most recently from f819b2b to d637008 Compare May 2, 2025 08:26
@mmorel-35 mmorel-35 marked this pull request as ready for review May 2, 2025 08:27
Copy link
Copy Markdown
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

As far as I know, since both 0755 and 0o755 compile to the exact same value in Go 1.13+, could you share your reasoning for preferring 0o? For example, is this driven by an updated style guideline or readability concerns? I’d love to understand the benefits so we can decide on a consistent convention for the project.

@mmorel-35
Copy link
Copy Markdown
Contributor Author

To my understanding it is for more clarity.
See :

This is recommended by go-critic and gofumpt, see "Octal integer literals should use the 0o prefix on modules using Go 1.13 and later" section

Copy link
Copy Markdown
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

Thanks for explaining. Since C doesn't support 0o prefix, I've never felt like 0 prefix is less clear. However, I don't mind either 0o or 0. We can follow it as some linters recommend 0o.

@knqyf263 knqyf263 added this pull request to the merge queue May 5, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 5, 2025
@knqyf263 knqyf263 added this pull request to the merge queue May 5, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 5, 2025
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
@mmorel-35 mmorel-35 force-pushed the go-critic/octalLiteral branch from d637008 to 46e18ea Compare May 5, 2025 11:27
@knqyf263 knqyf263 added this pull request to the merge queue May 5, 2025
Merged via the queue into aquasecurity:main with commit a19e0aa May 5, 2025
12 checks passed
@aqua-bot aqua-bot mentioned this pull request May 5, 2025
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