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: improve grammar in writingchecks.xml #11379

Merged
merged 1 commit into from Apr 8, 2022

Conversation

OttoKaaij
Copy link
Contributor

While reading through this document to get started on more substantial contributions, I decided to attempt to fix some grammatical errors present in the document. The content of the text was very helpful, so big thanks there to the writers!

@romani
Copy link
Member

romani commented Mar 8, 2022

Make commit message with prefix "doc: "

@OttoKaaij OttoKaaij closed this Mar 8, 2022
@OttoKaaij OttoKaaij reopened this Mar 8, 2022
@OttoKaaij OttoKaaij force-pushed the writingchecks-grammar branch 2 times, most recently from 4da3044 to 6edfbaf Compare March 8, 2022 15:49
@OttoKaaij
Copy link
Contributor Author

Oops, sorry: 'doc' is not 'Doc'

@romani
Copy link
Member

romani commented Mar 9, 2022

GitHub, generate web site

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2022

Copy link
Member

@rnveach rnveach left a comment

Choose a reason for hiding this comment

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

Half reviewed the changes.

src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Show resolved Hide resolved
@rnveach
Copy link
Member

rnveach commented Mar 12, 2022

@OttoKaaij I just want to check if there is any issues with making updates before I review the second half. I am waiting before I dive into the rest.

@OttoKaaij
Copy link
Contributor Author

@rnveach No problems! Intending to work through your reveiw tomorow. Thanks for your review!

@OttoKaaij OttoKaaij force-pushed the writingchecks-grammar branch 2 times, most recently from 7f9864a to 0e8e434 Compare March 13, 2022 09:46
@romani
Copy link
Member

romani commented Mar 13, 2022

@OttoKaaij, please review CI failures and fix them.

@OttoKaaij
Copy link
Contributor Author

@OttoKaaij, please review CI failures and fix them.

Should be fixed

@rnveach
Copy link
Member

rnveach commented Mar 22, 2022

The PR is based on a master that is 31 commit(s) old.

Please make sure you are rebasing this PR on the latest version of master. Don't create a merge commit, it has to be a rebase.

@rnveach
Copy link
Member

rnveach commented Mar 22, 2022

I will find time today to review the second half.

src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
src/xdocs/writingchecks.xml Outdated Show resolved Hide resolved
@romani
Copy link
Member

romani commented Mar 23, 2022

@OttoKaaij, please don't do more changes.
Please apply all suggestions from rnveach.
If you disagree, revert this change and we will discuss it in separate PR.

Big PRs are not pleasant to review and manage.

@OttoKaaij
Copy link
Contributor Author

Resolved all comments and rebased

@romani
Copy link
Member

romani commented Mar 23, 2022

@OttoKaaij, in future PRs please do not resolve review items , just reply "done". Reviewer will resolve them as confirmation that update is done as requested.

@OttoKaaij
Copy link
Contributor Author

@romani that seems reasonable, thanks for letting me know :)

@romani
Copy link
Member

romani commented Mar 24, 2022

@rnveach , please finish review

@rnveach
Copy link
Member

rnveach commented Mar 30, 2022

@OttoKaaij There are 2 items from my first review that are still not done. Make sure you are showing any hidden conversations.
#11379 (comment)

@rnveach rnveach assigned romani and unassigned rnveach Mar 30, 2022
@rnveach
Copy link
Member

rnveach commented Mar 30, 2022

@romani Please review as well. I need a moment to review it again.

@romani
Copy link
Member

romani commented Apr 2, 2022

GitHub, generate website

Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

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

I am ok with changes

@romani romani assigned rnveach and unassigned romani Apr 2, 2022
@OttoKaaij
Copy link
Contributor Author

@rnveach I have fixed the final two comments you have up

@rnveach
Copy link
Member

rnveach commented Apr 7, 2022

@OttoKaaij One more thing, CI is failing because this PR is so old. Please rebase on the latest master and then it can be merged safely.

The PR is based on a master that is 41 commit(s) old. The max allowed is 10.

@romani
Copy link
Member

romani commented Apr 7, 2022

GitHub, rebase

@romani
Copy link
Member

romani commented Apr 7, 2022

manually rebased

@romani romani merged commit a03495c into checkstyle:master Apr 8, 2022
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.

None yet

3 participants