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

bluetooth: samples: grammatical fixes #82673

Closed
wants to merge 1 commit into from

Conversation

baaatli
Copy link

@baaatli baaatli commented Dec 6, 2024

Fixed some of the grammatical errors and spelling mistakes in the README.rst file.

@kartben
Copy link
Collaborator

kartben commented Dec 6, 2024

@baaatli is there a reason for closing/reopening new PRs? Anything we can do to help?

@baaatli
Copy link
Author

baaatli commented Dec 6, 2024

Hi @kartben

Sorry, noobie Zephyrer here :)

I am learning to contribute on Zephyr. I missed some of the checks in the last PR I raised. I did not realise the things like ` before the reference links and hence closed the previous PRs to not waste your time reviewing it. I'll make sure to review them better before raising a PR the next time. Apologies for the inconvenience.

@kartben
Copy link
Collaborator

kartben commented Dec 6, 2024

No worries:)
You can just keep your PR in draft mode until it's "green" and ready to be reviewed. You can force push amended commit to tour branch, no need to close/reopen all the time :)

@dipakgmx
Copy link
Member

dipakgmx commented Dec 6, 2024

Hi @baaatli, could you please squash your commits.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Dec 7, 2024

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@0ea9c65 zephyrproject-rtos/hal_nxp@d291bdc (master) zephyrproject-rtos/[email protected]

All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@baaatli
Copy link
Author

baaatli commented Dec 7, 2024

@dipakgmx done!

( some mishap in between, sorry :) )

Copy link
Member

@jhedberg jhedberg left a comment

Choose a reason for hiding this comment

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

Looks generally fine to me - just a small whitespace issue I commented on separately inline

@jhedberg
Copy link
Member

jhedberg commented Dec 7, 2024

Please squash the two commits into a single one

kartben
kartben previously approved these changes Dec 7, 2024
Fixed some grammatical and spelling mistakes in channel sounding
example

Signed-off-by: Vardhan Batavia <[email protected]>
revision: 0ea9c65e8e14fe01c055e571ad2e6fe5337dc58d
revision: d291bdcc4a59bace5ae7453e777e06080ccda8ce
Copy link
Member

Choose a reason for hiding this comment

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

What's this? Doesn't seem related to any language fixes.

Copy link
Author

Choose a reason for hiding this comment

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

idk where this stemmed from. I didn't change anything in this file.

I did this to squash commits. Maybe this squashed a different commit.

git fetch git reset --soft HEAD~2 git commit git push -f

Copy link
Author

Choose a reason for hiding this comment

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

I think this is too much effort for you folks to have so many reviews for such a small change. I will close this PR, raise a new one in draft mode and make sure it passes the pipeline before raising it for your review. Thanks for your time.

Copy link
Member

Choose a reason for hiding this comment

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

You can drop that commit and then the PR is good.

Copy link
Member

Choose a reason for hiding this comment

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

@baaatli when you have just a single commit, you can update it simply using git commit --amend. You can always check the content what you're about to push using e.g. git show (for just a single commit) or e.g. git log -p upstream/main... Also, no need to close this PR and create a new one. Just fix up your local tree, and once you've verified that everything is fine do another git push --force.

@baaatli baaatli closed this Dec 8, 2024
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