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

[clang-format] disable for all arrays in LangCodeExpander.cpp #17669

Merged
merged 1 commit into from
Apr 16, 2020
Merged

[clang-format] disable for all arrays in LangCodeExpander.cpp #17669

merged 1 commit into from
Apr 16, 2020

Conversation

howie-f
Copy link
Contributor

@howie-f howie-f commented Apr 12, 2020

Description

follow-up pr to #17626
we need to disable clang-format on all arrays instead of only the last one

Motivation and Context

see https://jenkins.kodi.tv/job/BuildMulti-PR/15513/artifact/PR17665.diff of
PR #17665

clang-format doesn't always work properly in those cases

Types of change

  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Cosmetic change (non-breaking change that doesn't touch code)

Checklist:

@fuzzard would you please have a quick look (real 1-liner this time)

@fuzzard
Copy link
Contributor

fuzzard commented Apr 12, 2020

understand the reasoning, but i think the approach is wrong.

I would look to wrap the individual arrays in an on/off. Reason being is if someone were to add something else, that just happened to be inside this large (~1000 LOC) on/off (think non array code potentially) then clang formatting wont apply, even though it should.

I know the likely hood is slim, but I lean more towards the explicit side of things at times, so i'll let others comment.

@howie-f
Copy link
Contributor Author

howie-f commented Apr 12, 2020

sounds reasonable

@fuzzard fuzzard added this to the Matrix 19.0-alpha 1 milestone Apr 12, 2020
@fuzzard fuzzard added Type: Improvement non-breaking change which improves existing functionality v19 Matrix labels Apr 12, 2020
@howie-f howie-f changed the title [clang] disable for all arrays in LangCodeExpander.cpp [clang-format] disable for all arrays in LangCodeExpander.cpp Apr 13, 2020
Copy link
Contributor

@phunkyfish phunkyfish left a comment

Choose a reason for hiding this comment

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

Looks Good

@phunkyfish phunkyfish added Type: Cleanup non-breaking change which removes non-working or unmaintained functionality and removed Type: Improvement non-breaking change which improves existing functionality labels Apr 16, 2020
@phunkyfish
Copy link
Contributor

Thanks @howie-f

@phunkyfish phunkyfish merged commit 280d628 into xbmc:master Apr 16, 2020
@howie-f
Copy link
Contributor Author

howie-f commented Apr 16, 2020

thanks @phunkyfish clang-format off was your hint 😉

@howie-f howie-f deleted the v19-disable-clang branch April 17, 2020 05:08
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 17, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 17, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 17, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 17, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 17, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 17, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 17, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 18, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request May 5, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 3, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 7, 2020
[clang-format] disable for all arrays in LangCodeExpander.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v19 Matrix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants