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

docs: fix spelling #1117

Merged
merged 2 commits into from
Apr 30, 2024
Merged

docs: fix spelling #1117

merged 2 commits into from
Apr 30, 2024

Conversation

jbampton
Copy link
Contributor

@jbampton jbampton commented May 7, 2021

Another lot of spelling fixes and this has 53 additions and some that are not in the other PRs

The other two PRs for typo fixes only have 28 and 21 additions.

refs #579
refs #675

Copy link
Contributor

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

Confirm that PR contains only valid spelling changes into single words - and nothing else. Not sure whether in this case we need explicit ACK by authors of all those BIPs – probably multiple ACKs from independent ppl will do the job better. What do you think, @kallewoof and @luke-jr?

@michaelfolkson
Copy link
Contributor

michaelfolkson commented May 8, 2021

I'm not a fan of these bulk spelling fixes across a large number of different BIPs in a single PR if each minor change to a BIP needs to be ACKed by the BIP champion(s).

I think going forward, we either allow minor spelling fixes to be merged without the BIP champion(s) being notified and ACKing them or we ask spelling fix PRs to only change one specific BIP at a time. The former would be a change to current BIP processes but perhaps could be included when we revise BIP processes at a future date.

edit: Found this previous PR from @luke-jr to allow BIP editors to merge minor spelling changes without ACKs from BIP champion(s) and concerns were raised. Hence I think we should ask spelling fix PRs to only change one specific BIP at a time and not bundle spelling fixes across BIPs into one PR.

@kallewoof
Copy link
Member

I think each PR should address a specific BIP and be approved by their respective authors.

@michaelfolkson
Copy link
Contributor

I think each PR should address a specific BIP and be approved by their respective authors.

I think these bulk spelling fix PRs across BIPs will need to be discouraged and eventually closed then. @luke-jr has also struggled to get BIP authors to care about minor spelling fixes. Thanks for the PR @jbampton but getting all the BIP champion(s) to ACK all the changes in a PR like this just isn't feasible.

@bitcoin bitcoin deleted a comment from daniel3997 Jun 13, 2021
@luke-jr
Copy link
Member

luke-jr commented Jul 2, 2021

@dr-orlovsky It would be nice if BIP editors could just merge spelling changes, but unfortunately the current process requires each author to ACK.

@kallewoof
Copy link
Member

@dr-orlovsky It would be nice if BIP editors could just merge spelling changes, but unfortunately the current process requires each author to ACK.

We should address this as part of the revised next process, IMO.

@bitcoin bitcoin deleted a comment Jan 22, 2022
@jonatack
Copy link
Contributor

@jbampton This pull needs rebase due to merge conflicts in bip_0174.mediawiki and bip_0330.mediawiki that need to be resolved.

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

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

All of the changes in this PR fix only typos within a word across multiple BIPs, they do not change the meaning of any BIP.

ACK 583e103

@murchandamus
Copy link
Contributor

I‘ve fixed the merge conflicts with BIP174 and BIP330 (where the passages in which the typo appeared had both been amended independently).

@murchandamus murchandamus merged commit f61885e into bitcoin:master Apr 30, 2024
3 checks passed
@jbampton jbampton deleted the fix-spelling branch May 6, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants