search

Samsung / ONE

On-device Neural Engine
Other
439 stars 157 forks source link

[record-hessian] RecordHessian resolve a compile issue. #14298

Closed BLee-bot closed 2 weeks ago

BLee-bot commented 3 weeks ago

This resolves compile issue.

ONE-DCO-1.0-Signed-off-by: Banseok Lee bs93.lee@samsung.com

Related Issue : https://github.com/Samsung/ONE/issues/13480 Draft PR: https://github.com/Samsung/ONE/pull/13585

seanshpark commented 3 weeks ago

plz correct commit message

current is

image

seanshpark commented 2 weeks ago

current commit message is

image

plz begin [recor...] tag and revise title/description with what this changes does.

BLee-bot commented 2 weeks ago

current commit message is

image

plz begin [recor...] tag and revise title/description with what this changes does.

Ok, will add [record-hessian] and more detailed descriptiuon at all commit message.

seanshpark commented 2 weeks ago

Your new message is

 [record-hessian] This commit add linking nncc_common to record-hessian.

This commit add linking nncc_common to record-hessian.

Add target_link_libraries(record-hessian nncc_common) in CMakeLists.txt

I wrote "what this changes does." in review comment.

What I see in this PR are;

Add target_link_libraries(record-hessian nncc_common) in CMakeLists.txt looks too detail.

If you have any more problems reading my comments, feel free to ask.

BLee-bot commented 2 weeks ago

Your new message is

 [record-hessian] This commit add linking nncc_common to record-hessian.

This commit add linking nncc_common to record-hessian.

Add target_link_libraries(record-hessian nncc_common) in CMakeLists.txt

I wrote "what this changes does." in review comment.

What I see in this PR are;

  • this changes does add linking nncc_common
  • this changes fixes errors with strict compilation option

Add target_link_libraries(record-hessian nncc_common) in CMakeLists.txt looks too detail.

If you have any more problems reading my comments, feel free to ask.

I appreciate your detailed guides. One thing I need to be sure is that.. You mean the first commit of this PR, right? Not all commits I pushed?

seanshpark commented 2 weeks ago

first commit of this PR, right?

Yes, first commit message will be used for this PR.

seanshpark commented 2 weeks ago 
This changes does add linking nncc_common
This changes fixes errors with strict compilation option

This wordings are from my point of view to describe what I understood, and I don't think these are for the commit message.