Skip to content

: Fix lint in clang-format #3041

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

Closed
wants to merge 1 commit into from

Conversation

mergennachin
Copy link
Contributor

@mergennachin mergennachin commented Apr 15, 2024

Summary:
We are updating to clang-formatter 18.

The current clang-format in coreml code has duplicate key. Deleting one of them. It didn't complain in the old clang-formatter 12

Differential Revision: D56139927

Copy link

pytorch-bot bot commented Apr 15, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/3041

Note: Links to docs will display an error until the docs builds have been completed.

❌ 3 New Failures

As of commit 88d244f with merge base 057e432 (image):

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56139927

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Apr 15, 2024
mergennachin added a commit to mergennachin/executorch-1 that referenced this pull request Apr 15, 2024
Summary:

We are updating to clang-formatter 18.

The current clang-format in coreml code has duplicate key. Deleting one of them.

Differential Revision: D56139927
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56139927

@mergennachin mergennachin requested a review from cymbalrush April 15, 2024 14:32
@@ -1,5 +1,4 @@
BasedOnStyle: WebKit
BreakBeforeBraces: Attach
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cymbalrush BreakBeforeBraces was defined twice in this class. With the new clang-format 18, it doesn't allow to have duplicate keys.

Deleting one of them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's an example error when running lintrunner

Advice (CLANGFORMAT) command-failed
  COMMAND (exit code 1)
  /home/mnachin/local/miniconda3/envs/executorch_docs/bin/clang-format
  -style=file /data/users/mnachin/fbsource/fbcode/executorch/backends/apple/
  coreml/runtime/util/objc_safe_cast.h
  STDERR
  /data/users/mnachin/fbsource/fbcode/executorch/backends/
  apple/coreml/.clang-format:23:1: error: duplicated mapping key
  'BreakBeforeBraces'
  BreakBeforeBraces: Custom
  ^~~~~~~~~~~~~~~~~
  Error reading /data/users/mnachin/fbsource/fbcode/executorch/backends/
  apple/coreml/.clang-format: Invalid argument
  STDOUT
  (empty)
  

Summary:
Pull Request resolved: pytorch#3041

We are updating to clang-formatter 18.

The current clang-format in coreml code has duplicate key. Deleting one of them.

Differential Revision: D56139927
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D56139927

Copy link
Contributor

@cymbalrush cymbalrush left a comment

Choose a reason for hiding this comment

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

Thank you!

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 7c81155.

@mergennachin mergennachin mentioned this pull request Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants