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: build/config/settings fix confusing descriptions #63725

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

yf13
Copy link
Contributor

@yf13 yf13 commented Oct 10, 2023

rearrange point 2 as part of point 1, then update order of following points

nordicjm
nordicjm previously approved these changes Oct 10, 2023
@nordicjm
Copy link
Collaborator

Though please fix the commit message, remove the link to the issue and fix the commit message as per https://docs.zephyrproject.org/latest/contribute/guidelines.html#commit-message-guidelines

@yf13 yf13 changed the title Update setting.rst to reduction confusions as discussed in #63665 doc: build/conffig/settings fix confusing descriptions Oct 10, 2023
@yf13
Copy link
Contributor Author

yf13 commented Oct 10, 2023

How can I continue editing the commit message from within web browser?

@nordicjm
Copy link
Collaborator

How can I continue editing the commit message from within web browser?

You need to do it from command line with git

Copy link
Collaborator

@tejlmand tejlmand 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 your contribution.
The changes looks good, but you need to rebase your commit and get rid of the merge-commit.

See more here: https://docs.zephyrproject.org/latest/contribute/guidelines.html#contribution-workflow

Copy link
Collaborator

@kartben kartben 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 PR!
Please see my comment, and use git commit --amend --author="Author Name <[email protected]>" to fix your current commit so that it has the same author as the one specified in the Signed-off-by entry. Based on how much you want to actually exercise your git skills w.r.t rebasing, and because it's a pretty small change, you might find it easier to reset your branch to main and work your way through the two commits by re-starting form scratch though.
Thanks again!

doc/build/kconfig/setting.rst Outdated Show resolved Hide resolved
yf13 added 2 commits October 11, 2023 11:25
use unnumbered list to align with guidelines

Signed-off-by: Yanfeng Liu <[email protected]>
make item#2 as part of item#1 to reduce confusions.

Signed-off-by: Yanfeng Liu <[email protected]>
@yf13
Copy link
Contributor Author

yf13 commented Oct 11, 2023

use git commit --amend --author="Author Name <[email protected]>" to fix your current commit so that it has the same author as the one specified in the Signed-off-by entry.

this is fixed in my latest push.

@yf13
Copy link
Contributor Author

yf13 commented Oct 11, 2023

Though please fix the commit message, remove the link to the issue and fix the commit message as per https://docs.zephyrproject.org/latest/contribute/guidelines.html#commit-message-guidelines

commit messages have been fixed in my latest push.

@yf13 yf13 requested a review from kartben October 11, 2023 03:42
@yf13 yf13 changed the title doc: build/conffig/settings fix confusing descriptions doc: build/config/settings fix confusing descriptions Oct 11, 2023
@yf13
Copy link
Contributor Author

yf13 commented Oct 12, 2023

@kartben @tejlmand it seems that there are two approvals for this PR so far. As I am not authorized to merge this PR, please teach what I can do to move ahead. Should I simply click "close with comment" button ? I guess it might just close the conversations which doesn't seem necessary.

@kartben kartben added this to the v3.5.0 milestone Oct 12, 2023
@kartben
Copy link
Collaborator

kartben commented Oct 12, 2023

@kartben @tejlmand it seems that there are two approvals for this PR so far. As I am not authorized to merge this PR, please teach what I can do to move ahead. Should I simply click "close with comment" button ? I guess it might just close the conversations which doesn't seem necessary.

Please don't close it! Next step is for one of our release engineers to do the merge, which should happen momentarily now that I have set the target milestone to 3.5.
Thanks!

@jhedberg jhedberg merged commit a275e45 into zephyrproject-rtos:main Oct 12, 2023
14 checks passed
@github-actions
Copy link

Hi @yf13!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants