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/ko: Add missing options in uftrace-replay.md #1957

Closed
wants to merge 5 commits into from

Conversation

sinsohi
Copy link
Contributor

@sinsohi sinsohi commented Sep 2, 2024

Identified and corrected discrepancies between the English and
Korean versions of uftrace-replay.md:

  1. The description of the --disable option in the Korean version
    differed from the English version. This has been correctly
    retranslated.

  2. Added the missing options --trace=STATE, --srcline, and
    --no-args to the Korean version.

  3. Included the 'task' and 'module' fields for the
    -f FIELD, --output-fields=FIELD option in the Korean version,
    which were previously omitted.

These changes bring the Korean documentation in line with the
English version, ensuring consistency and completeness.

Signed-off-by: Sin Sohi sohi8451@naver.com

Identified and corrected discrepancies between the English and Korean versions of uftrace-replay.md:

1. The description of the --disable option in the Korean version differed from the English version. This has been correctly retranslated.

2. Added the missing options --trace=*STATE*, --srcline, and --no-args to the Korean version.

3. Included the 'task' and 'module' fields for the -f *FIELD*, --output-fields=*FIELD* option in the Korean version, which were previously omitted.

These changes bring the Korean documentation in line with the English version, ensuring consistency and completeness.

Signed-off-by: Sinsohi <sohi8451@naver.com>
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 for the patch. I've just left one minor comment and others are fine.

But could you please enhance your commit message? I've asked many times about rewriting commit messages in various PRs so you can get some ideas.

The most recent request can be found at #1946 (review). Please read the comment and other comments below so that you can get more ideas what I have been asking before merge a PR.

I can see your commit message at https://github.com/namhyung/uftrace/pull/1957.patch.

It would be helpful if @gichoel or @ParkSeungHyeok can help him write better commit messages.

Comment on lines 122 to 123
: uftrace 트레이싱 상태를 설정한다. 가능한 상태는 `on`과 `off`이다. 기본값은
`on`이다. 이는 `trace_on` 트리거나 에이전트와 함께 사용할 때만 의미가 있다.
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about making it as follows?

Suggested change
: uftrace 트레이싱 상태를 설정한다. 가능한 상태는 `on``off`이다. 기본값은
`on`이다. 이는 `trace_on` 트리거나 에이전트와 함께 사용할 때만 의미가 있다.
: uftrace 트레이싱 상태를 설정한다. 가능한 상태는 `on``off` 이며 기본값은
`on`이다. 이는 `trace_on` 트리거나 에이전트와 함께 사용할 때만 의미가 있다.

@gichoel
Copy link
Contributor

gichoel commented Sep 3, 2024

First, thanks for your contribution.

However, there are some rules for writing a good readable commit message, and the commit message improvement that honggyukim is talking about seems to be related to the 50/72 rule[1].

In the case of the commit message currently written, the subject does not exceed 50 characters, so it satisfies the rule, but the body does not satisfies the rule because it is not broken every 72 characters.

Could you please make this change?

[1] https://www.gitkraken.com/learn/git/best-practices/git-commit-message#git-commit-message-structure

Identified and corrected discrepancies between the English and
Korean versions of uftrace-replay.md:

1. The description of the --disable option in the Korean version
   differed from the English version. This has been correctly
   retranslated.

2. Added the missing options --trace=*STATE*, --srcline, and
   --no-args to the Korean version.

3. Included the 'task' and 'module' fields for the
   -f *FIELD*, --output-fields=*FIELD* option in the Korean version,
   which were previously omitted.

These changes bring the Korean documentation in line with the
English version, ensuring consistency and completeness.

Signed-off-by: Sinsohi <sohi8451@naver.com>
@sinsohi
Copy link
Contributor Author

sinsohi commented Sep 4, 2024

Thank you for your feedback.
I've updated the commit message and code as requested:

  1. Improved Korean translation for the --trace=STATE option.
  2. Adjusted the commit message format following the 50/72 rule.

Let me know if any further modifications are needed.

@honggyukim
Copy link
Collaborator

Please update your name correctly as I mentioned at #1957 (review).

The most recent request can be found at #1946 (review).

Identified and corrected discrepancies between the English and
Korean versions of uftrace-replay.md:

1. The description of the --disable option in the Korean version
   differed from the English version. This has been correctly
   retranslated.

2. Added the missing options --trace=*STATE*, --srcline, and
   --no-args to the Korean version.

3. Included the 'task' and 'module' fields for the
   -f *FIELD*, --output-fields=*FIELD* option in the Korean version,
   which were previously omitted.

These changes bring the Korean documentation in line with the
English version, ensuring consistency and completeness.

Signed-off-by: Sin Sohi <sohi8451@naver.com>
@namhyung
Copy link
Owner

namhyung commented Sep 4, 2024

Please don't merge, just rebase your changes on top of the current master.

@sinsohi
Copy link
Contributor Author

sinsohi commented Sep 4, 2024

Thank you for your feedback.
I have reflected your requirements.
Please check it out.

@honggyukim
Copy link
Collaborator

Please squash all the commits in this PR. And it has conflict with the master so can't be merged.

Identified and corrected discrepancies between the English and
Korean versions of uftrace-replay.md:

1. The description of the --disable option in the Korean version
   differed from the English version. This has been correctly
   retranslated.

2. Added the missing options --trace=*STATE*, --srcline, and
   --no-args to the Korean version.

3. Included the 'task' and 'module' fields for the
   -f *FIELD*, --output-fields=*FIELD* option in the Korean version,
   which were previously omitted.

These changes bring the Korean documentation in line with the
English version, ensuring consistency and completeness.

Signed-off-by: Sin Sohi <sohi8451@naver.com>

mcount: Get rid of tid from retstack

I don't remember why it has tid for each return stack entry.  It's not
used so let's get rid of it.  Also move dyn_idx field to remove the
padding.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>

mcount: Handle recover trigger only if -e is not used

The -e/--estimate-return doesn't hook the return addresses so no need to
recover the original address.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>

mcount: Use mcount_memcpy1() instead of memcpy()

We don't want to use external memcpy() function in the mcount fast path
because it might use some SSE/AVX registers internally.  We want to
preserve the original register values and to avoid saving and restoring
all the registers unnecessarily.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>

doc/ko: Add missing options in uftrace-replay.md

Identified and corrected discrepancies between the English and
Korean versions of uftrace-replay.md:

1. The description of the --disable option in the Korean version
   differed from the English version. This has been correctly
   retranslated.

2. Added the missing options --trace=*STATE*, --srcline, and
   --no-args to the Korean version.

3. Included the 'task' and 'module' fields for the
   -f *FIELD*, --output-fields=*FIELD* option in the Korean version,
   which were previously omitted.

These changes bring the Korean documentation in line with the
English version, ensuring consistency and completeness.

Signed-off-by: Sinsohi <sohi8451@naver.com>
@sinsohi
Copy link
Contributor Author

sinsohi commented Sep 4, 2024

I'm having trouble resolving this. Would it be okay if I delete this PR and submit a new one

@sinsohi sinsohi closed this Sep 4, 2024
@honggyukim
Copy link
Collaborator

Please update it here and don't open a new PR. If you're a contribution academy mentee, then you can ask @gichoel.

@sinsohi
Copy link
Contributor Author

sinsohi commented Sep 4, 2024

I apologize. I didn't check your response and, due to severe errors, I hastily closed the PR and deleted the repository. I'm really sorry, but would it be alright if I fork again and resend the PR?

@honggyukim
Copy link
Collaborator

You don't have to say sorry. We know how it is difficult at the first try. The issue is that I am quite busy these days so cannot explain all the details.

I just hope someone else can help you with this issue because helping new contributors is also a great contribution to the project.

@gichoel
Copy link
Contributor

gichoel commented Sep 4, 2024

sinsohi is not a Contribution Academy mentee, so I am limited in helping directly, but I'd like to help new contributors get acclimated to uftrace.

However, after checking the commit hash of the PR and the commit hash of sinsohi's forked repository, it seems that the repository was already deleted, recreated, and then the commits were uploaded again.

@honggyukim
From what I have investigated, it seems that the PR cannot be reopened in this case, so would it be correct to create a new PR and then mention the this PR?

@honggyukim
Copy link
Collaborator

Sorry @gichoel. I misunderstood that.

@sinsohi Do you mind if I modify your patch rather than asking many requests? If you're okay, then I can just merge it into the master directly without creating a new PR.

@sinsohi
Copy link
Contributor Author

sinsohi commented Sep 5, 2024

I would be really grateful if you could do that.

@honggyukim
Copy link
Collaborator

Thanks. I will do that when I have some time.

@honggyukim
Copy link
Collaborator

@sinsohi I have a simple question. What do you want me to write your name among the followings?

  1. Sin Sohi
  2. Shin Sohi
  3. Sohi Sin
  4. Sohi Shin

@sinsohi
Copy link
Contributor Author

sinsohi commented Sep 11, 2024

@honggyukim Please write my name as Sin Sohi. Thank you.

@honggyukim
Copy link
Collaborator

@sinsohi Sorry, I've been busy for a while. I will do that within a few days.

@honggyukim
Copy link
Collaborator

Hi @sinsohi, sorry for making delay. I've applied your patch at 5430d02 and it's now in master. Thanks for your work!

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.

4 participants