[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

[lint] Enforce k prefix for global constants #33666

Merged
merged 5 commits into from
May 28, 2022
Merged

[lint] Enforce k prefix for global constants #33666

merged 5 commits into from
May 28, 2022

Conversation

cbracken
Copy link
Member

Enforces that all global constants are prefixed with a 'k' as per the
style guide and updates the codebase into conformance where necessary.

This does not change any public API (or touch even private header
files).

Additional testing provided by the addition of the lint rule.

Ref: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#begin-global-constant-names-with-prefix-k

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added embedder Related to the embedder API platform-macos labels May 27, 2022
@cbracken
[lint] Enforce k prefix for global constants
f538d18 
Enforces that all global constants are prefixed with a 'k' as per the
style guide and updates the codebase into conformance where necessary.

This does not change any public API (or touch even private header
files).

Additional testing provided by the addition of the lint rule.

Ref: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#begin-global-constant-names-with-prefix-k
@cbracken cbracken requested a review from jmagman May 27, 2022 21:28
jmagman
jmagman reviewed May 27, 2022
View reviewed changes
display_list/display_list_canvas_unittests.cc Outdated Show resolved Hide resolved
lib/ui/text/paragraph_builder.cc Outdated Show resolved Hide resolved
jmagman
jmagman approved these changes May 27, 2022
View reviewed changes
Copy link
Member
@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

Thanks, Chris!

@cbracken
Remove k prefix for consts in public API
547f14e 
We're stuck with these; bummer.
@cbracken cbracken merged commit 1c09bf1 into flutter:main May 28, 2022
@cbracken cbracken deleted the enforce-k-constants branch May 28, 2022 01:33
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 28, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 28, 2022
@engine-flutter-autoroll
1c09bf1 [lint] Enforce k prefix for global constants (flutter/engine#…
85f467e 
…33666)
cbracken added a commit to cbracken/flutter_engine that referenced this pull request May 28, 2022
@cbracken
[impeller] Revert ShaderLibraryGLES validity check
47d6149 
In flutter#33666, I landed a bulk lint
cleanup that applied fixes to global constant naming using the `--fix`
option to the linter script, but also caught a few straggling linter
violations that hadn't been previously fixed.

As part of this patch, the linter applied a change that moved
ShaderLibraryGLES's is_valid_ initialisation from the ctor body to the
initialiser list, always initialised to true. This is unsafe because
prior to is_valid_ initialisation in the ctor body, there was a bailout
if any of the shader_libraries wasn't valid.
@cbracken cbracken mentioned this pull request May 28, 2022
Merged
8 tasks
cbracken added a commit to cbracken/flutter_engine that referenced this pull request May 28, 2022
@cbracken
[impeller] Revert ShaderLibraryGLES validity check
88b250f 
In flutter#33666, I landed a bulk lint
cleanup that applied fixes to global constant naming using the `--fix`
option to the linter script, but also caught a few straggling linter
violations that hadn't been previously fixed.

As part of this patch, the linter applied a change that moved
ShaderLibraryGLES's is_valid_ initialisation from the ctor body to the
initialiser list, always initialised to true. This is unsafe because
prior to is_valid_ initialisation in the ctor body, there was a bailout
if any of the shader_libraries wasn't valid.

This eliminates the use of the
cppcoreguidelines-prefer-member-initializer lint, which incorrectly
flagged this code due to the following issue:
llvm/llvm-project#52837
cbracken added a commit to cbracken/flutter_engine that referenced this pull request May 28, 2022
@cbracken
[impeller] Revert ShaderLibraryGLES validity check
c83e22d 
In flutter#33666, I landed a bulk lint
cleanup that applied fixes to global constant naming using the `--fix`
option to the linter script, but also caught a few straggling linter
violations that hadn't been previously fixed.

As part of this patch, the linter applied a change that moved
ShaderLibraryGLES's is_valid_ initialisation from the ctor body to the
initialiser list, always initialised to true. This is unsafe because
prior to is_valid_ initialisation in the ctor body, there was a bailout
if any of the shader_libraries wasn't valid.

This eliminates the use of the
cppcoreguidelines-prefer-member-initializer lint, which incorrectly
flagged this code due to the following issue:
llvm/llvm-project#52837

This is a partial revert of flutter#33666
cbracken added a commit that referenced this pull request May 29, 2022
@cbracken
[impeller] Revert ShaderLibraryGLES validity check (#33679)
e28e576 
In #33666, I landed a bulk lint
cleanup that applied fixes to global constant naming using the `--fix`
option to the linter script, but also caught a few straggling linter
violations that hadn't been previously fixed.

As part of this patch, the linter applied a change that moved
ShaderLibraryGLES's is_valid_ initialisation from the ctor body to the
initialiser list, always initialised to true. This is unsafe because
prior to is_valid_ initialisation in the ctor body, there was a bailout
if any of the shader_libraries wasn't valid.

This eliminates the use of the
`cppcoreguidelines-prefer-member-initializer` lint, which incorrectly
flagged this code due to the following issue:
llvm/llvm-project#52837

This is a partial revert of #33666.
Reintroduction of this issue is prevented by the removal of the 
`cppcoreguidelines-prefer-member-initializer` lint in this patch.
@jmagman jmagman mentioned this pull request May 31, 2022
Closed
houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022
@cbracken @houhuayong
[lint] Enforce k prefix for global constants (flutter#33666)
bcdb0f9 
Enforces that all global constants are prefixed with a 'k' as per the
style guide and updates the codebase into conformance where necessary.

This does not change any public API.

Additional testing provided by the addition of the lint rule.

Ref: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#begin-global-constant-names-with-prefix-k
houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022
@cbracken @houhuayong
[impeller] Revert ShaderLibraryGLES validity check (flutter#33679)
1f0eb01 
In flutter#33666, I landed a bulk lint
cleanup that applied fixes to global constant naming using the `--fix`
option to the linter script, but also caught a few straggling linter
violations that hadn't been previously fixed.

As part of this patch, the linter applied a change that moved
ShaderLibraryGLES's is_valid_ initialisation from the ctor body to the
initialiser list, always initialised to true. This is unsafe because
prior to is_valid_ initialisation in the ctor body, there was a bailout
if any of the shader_libraries wasn't valid.

This eliminates the use of the
`cppcoreguidelines-prefer-member-initializer` lint, which incorrectly
flagged this code due to the following issue:
llvm/llvm-project#52837

This is a partial revert of flutter#33666.
Reintroduction of this issue is prevented by the removal of the 
`cppcoreguidelines-prefer-member-initializer` lint in this patch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chinmaygarde chinmaygarde chinmaygarde approved these changes

@jmagman jmagman jmagman approved these changes

Assignees
No one assigned
Labels
embedder Related to the embedder API platform-macos
Projects
None yet
Milestone
No milestone
3 participants
@cbracken @chinmaygarde @jmagman