Skip to content

Conversation

@vmoroz
Copy link
Member

@vmoroz vmoroz commented Jan 14, 2021

I am getting clang-format crashes when editing JSValueTests.cpp.
Our clang-format version is very old.
I have updated it to 1.5.0 and I do not see crashes anymore.
It also produces much better formatting results, but the changes resulted in a lot of files changed due to reformatting.

Note: No changes are done to files except for the automated changes by calling yarn format.

The clang-format is added to the root package.json to be automatically updated in future.

Microsoft Reviewers: Open in CodeFlow

@vmoroz vmoroz requested review from a team, NickGerleman, acoates-ms and dannyvv January 14, 2021 19:21
@vmoroz vmoroz requested a review from a team as a code owner January 14, 2021 19:21
Copy link
Contributor

@NickGerleman NickGerleman left a comment

Choose a reason for hiding this comment

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

Change looks good to me, but please remove the dependency from the root package.json before merging.

Copy link
Member

@dannyvv dannyvv left a comment

Choose a reason for hiding this comment

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

Looks like most of the changes the new version brought in are better,
Still clang format remains to have some baffling formatting behavior to me :)

@vmoroz vmoroz merged commit fed79b5 into microsoft:master Jan 14, 2021
vmoroz added a commit that referenced this pull request Jan 15, 2021
* Fix unused parameter warning in AbiCallInvoker (#6860)

* Move CallInvoker.h to TurboModule filter folder (#6861)

* Simplify Microsoft.ReactNative.IntegrationTests (#6868)

* Update clang-format version to 1.5.0 (#6870)

* Add JSValue constructor for std::optional<T> (#6873)

* Updated doc comments in IReactNotificationService.idl (#6875)

* Fix ReactNotificationService for notifications between app and modules (#6877)
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.

3 participants