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

doc: Modified README.md and ko/README.md #1027

Merged
merged 1 commit into from
Nov 14, 2019
Merged

doc: Modified README.md and ko/README.md #1027

merged 1 commit into from
Nov 14, 2019

Conversation

kangtegong
Copy link
Contributor

@kangtegong kangtegong commented Nov 9, 2019

Since Lua 5.1 script is now available and
dynamic tracing for AArch64 (ARM64) is possible in uftrace,
this commit updates both English README and Korean README
as containing those information.

Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

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

AArch53?

README.md Outdated
@@ -88,7 +88,7 @@ For recording, the executable needs to be compiled with the `-pg`
(or `-finstrument-functions`) option which generates profiling code
(calling mcount or __cyg_profile_func_enter/exit) for each function.

Note that, there's an experimental support for dynamic tracing on x86_64
Note that, there's an experimental support for dynamic tracing on x86_64 and AArch64 (ARM64)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do not go over 80 characters in a single line.

@honggyukim
Copy link
Collaborator

Please change the commit message from AArch53 to AArch64.

@kangtegong
Copy link
Contributor Author

amend commit message from AArch53 to AArch64
and
modified line breaks so that there's less than 80 words in one line

@kangtegong
Copy link
Contributor Author

kangtegong commented Nov 11, 2019

AArch53?

My mistake..! I amended the commit message

README.md Outdated
(although it still needs recompilation of your program). Please see
[doc/uftrace-record.md](doc/uftrace-record.md) file.
Note that, there's an experimental support for dynamic tracing on
x86_64 and AArch64 (ARM64) which doesn't require such (re-)compilations.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please change "AArch64 (ARM64)" to "AArch64(ARM64)".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please change "AArch64 (ARM64)" to "AArch64(ARM64)".

Okay!

@kangtegong
Copy link
Contributor Author

changed AArch64 (ARM64) to AArch64(ARM64)

README.md Outdated
(although it still needs recompilation of your program). Please see
[doc/uftrace-record.md](doc/uftrace-record.md) file.
Note that, there's an experimental support for dynamic tracing on
x86_64 and AArch64(ARM64) which doesn't require such (re-)compilations.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please change it as follows:

Suggested change
x86_64 and AArch64(ARM64) which doesn't require such (re-)compilations.
x86_64 and AArch64(ARM64) which do not require such (re-)compilations.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not changed yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I forgot, I'll amend right away :)

Also recent compilers have some options to help uftrace
to reduce tracing overhead with similar way
(although it still needs recompilation of your program).
Please see [doc/uftrace-record.md](doc/uftrace-record.md) file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove all the white spaces at the end of lines. They are shown as red blocks in the diff output.
Screen Shot 2019-11-11 at 11 22 04 PM

@kangtegong
Copy link
Contributor Author

kangtegong commented Nov 11, 2019

removed white spaces at the end of lines!

README.md Outdated
(although it still needs recompilation of your program). Please see
[doc/uftrace-record.md](doc/uftrace-record.md) file.
Note that, there's an experimental support for dynamic tracing on
x86_64 and AArch64(ARM64) which do not require such (re-)compilations.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm sorry for asking you again. I was wrong. "which doesn't require" is actually correct.
Everything looks good except for it. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry for asking you again. I was wrong. "which doesn't require" is actually correct.
Everything looks good except for it. Thanks.

It's okay :) I'll fix it right away!

Since Lua 5.1 script is now available and
dynamic tracing for AArch64 (ARM64) is possible in uftrace,
this commit updates both English README and Korean README
as containing those information.
@kangtegong
Copy link
Contributor Author

Fixed which do not require to which doesn't require

Copy link
Collaborator

@honggyukim honggyukim left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

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

LGTM

@namhyung namhyung merged commit 5ccbf95 into namhyung:master Nov 14, 2019
@kangtegong kangtegong deleted the README branch November 14, 2019 08:07
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