Page MenuHomePhabricator
Differential D4761

[Services] Clang fixes for services/lib/src
ClosedPublic
Actions

Authored by • jon on Aug 5 2022, 4:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jun 23, 4:41 PM
Unknown Object (File)
Sun, Jun 23, 4:41 PM
Unknown Object (File)
Sun, Jun 23, 4:41 PM
Unknown Object (File)
Sun, Jun 23, 4:29 PM
Unknown Object (File)
Fri, Jun 21, 7:34 PM
Unknown Object (File)
Thu, Jun 20, 10:18 PM
Unknown Object (File)
Thu, Jun 20, 6:39 PM
Unknown Object (File)
Wed, Jun 19, 12:23 PM
View All 77 Files
Subscribers
• abosh
• adrian
ashoat
tomek

Details

Reviewers
atul
• karol
• max
Commits
rCOMM4aef791d3ab1: [Services] Clang fixes for services/lib/src
Summary

Clang is a bit more pedantic. Make it happy.

Occurred to me that this was never an existing workflow.
So I made it into another diff.

Allows for D4489 to be built from a MacOS device

Test Plan

Follow nix test plan from D4489 on a mac device

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

• jon created this revision.Aug 5 2022, 4:40 PM
Herald added subscribers: • abosh, • adrian, tomek, ashoat. · View Herald TranscriptAug 5 2022, 4:40 PM
• jon added a child revision: D4489: [Services] Export shared services cpp as CMake Project.Aug 5 2022, 4:41 PM
Harbormaster completed remote builds in B11180: Diff 15387.Aug 5 2022, 4:52 PM
• jon requested review of this revision.Aug 5 2022, 4:52 PM
• jon mentioned this in D4489: [Services] Export shared services cpp as CMake Project.Aug 5 2022, 6:20 PM
• jon retitled this revision from [Services] Clang fixes services/lib/src to [Services] Clang fixes for services/lib/src.Aug 6 2022, 10:17 AM
atul accepted this revision.Aug 8 2022, 10:29 AM
atul added inline comments.
services/lib/src/ReactorStatusHolder.h
25 ↗(On Diff #15387)

Could we do

std::atomic<ReactorState> state{ReactorState::NONE};

here?

Screen_Shot_2022-08-04_at_4.29.45_PM.png (1×902 px, 226 KB)

This revision is now accepted and ready to land.Aug 8 2022, 10:29 AM
• jon requested review of this revision.Aug 8 2022, 10:55 AM
• jon added inline comments.
services/lib/src/ReactorStatusHolder.h
25 ↗(On Diff #15387)

I'm not going to act like I know best practices in C++

cc @max

Looks like they are equivalent: https://stackoverflow.com/questions/40378205/c-initialising-fields-directly-vs-initialisation-list-in-default-constructor

atul added inline comments.Aug 8 2022, 11:03 AM
services/lib/src/ReactorStatusHolder.h
25 ↗(On Diff #15387)

Gotcha, seems reasonable to leave as is

Also don't know much C++, just been blindly following that braced initialization rule.

atul accepted this revision.Aug 8 2022, 11:04 AM
atul attached a referenced file: F131041: Screen_Shot_2022-08-04_at_4.29.45_PM.png. (Show Details)
This revision is now accepted and ready to land.Aug 8 2022, 11:04 AM
Closed by commit rCOMM4aef791d3ab1: [Services] Clang fixes for services/lib/src (authored by • jon). · Explain WhyAug 8 2022, 11:34 AM
This revision was automatically updated to reflect the committed changes.
• jon added a commit: rCOMM4aef791d3ab1: [Services] Clang fixes for services/lib/src.

Revision Contents
Changeset List

PathSize
services/
lib/
src/
2 lines
server-base-reactors/
2 lines

Diff 15436

View Options

services/lib/src/ReactorStatusHolder.h

Loading...
View Options

services/lib/src/server-base-reactors/ServerWriteReactorBase.h

Loading...