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

Spelling #71464

Closed
wants to merge 606 commits into from
Closed

Spelling #71464

wants to merge 606 commits into from

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Oct 12, 2021

This PR corrects misspellings identified by the check-spelling action.

The misspellings have been reported at jsoref@61dae6e

The action reports that the changes in this PR would make it happy: jsoref@ba9925f

Notes:

  • this PR does not include the action. If you're interested in running a spell check on every PR and push, that can be offered separately.

  • this PR exceeds 250 commits which means GitHub's classic PR viewer won't make it easy to review. The new VSC view should make it easier to review.
    I fully expect to split this in some way, I'm open to input on the how. It's generally easiest for me to split by file name/path/extension, however within limits, I can split by content (it's considerably more expensive to do so). I try to avoid squashing until near the end. I've so far done 4-5 rebases, most have ~1 conflict, and generally the typos carryover.

  • I will try to annotate the interesting bits (please bare with me, this will take a while)

  • The initial round of word replacements were suggested by Google Sheets, but all mistakes are obviously my responsibility, and I will fix them.

Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
Signed-off-by: Josh Soref <jsoref@users.noreply.github.com>
@irfansharif
Copy link
Contributor

Thanks for the PR @jsoref! Briefly looking through the diff, I'm seeing some changes to internal scripts and such that would merit a closer inspection. I find it unlikely we'll find reviewers to scrutinize every bit. We've also historically shied away from adding linters to our CI to catch misspellings -- we've found it to add little value, and our reviewers usually (but not always evidently, ha) pick out the egregious ones. On these accounts, I'm going to go ahead and close this PR.

@knz
Copy link
Contributor

knz commented Oct 12, 2021

@jsoref one more constructive way to do this is as follows:

  • create one PR per area of the code base (e.g. one per group of Go packages that are related to one area of the product)
  • double and triple check that the changes only affect comments and docs, and do not affect identifiers, directory names etc in code (your PR currently changes things in code and thus makes the PR invalid)

@irfansharif
Copy link
Contributor

the changes only affect comments and docs, and do not affect identifiers, directory names etc in code

+1. I'd also be happy to review a lump-sum single commit PR across the entire project assuming that's easier (as long as we're not making any code-changes).

@knz
Copy link
Contributor

knz commented Oct 12, 2021

I'd also be happy to review a lump-sum single commit PR across the entire project assuming that's easier

NB: we'll need to do the double- and triple-checking that there are no code changes on our side too, and so breaking down the PR will make the review easier to divvy up between multiple team members.

@irfansharif
Copy link
Contributor

(Was just saying I'm happy to do it myself -- it's a digestible 1k line diff.)

@jordanlewis
Copy link
Member

Hi @jsoref, like we discussed offline, I think it'll be easier to get this merged if we split it up by category. Some categories are a lot less controversial than others.

Things we like:

  1. Spelling fixes to RFCS or tech-notes, or other text-only files
  2. Spelling fixes to user-visible strings in things like dashboards or HTML

Things we're more neutral on:

  1. Spelling fixes to code comments

Things we don't like unless there's a strong reason:

  1. Spelling fixes to code itself, since it can make the blame more confusing

Things we can't accept:

  1. Spelling fixes to protobuf field names, schemas, baked test cases, etc

Thanks!

craig bot pushed a commit that referenced this pull request Oct 14, 2021
71519: docs/rfcs: correct spelling mistakes r=irfansharif a=jsoref

This PR corrects misspellings identified by the [check-spelling action](https://github.com/marketplace/actions/check-spelling).

This PR is limited to just the RFC directory as suggested in:
#71464 (comment)

* I try to avoid squashing until near the end.
* I will try to annotate the interesting bits (please bare with me, this will take a while)

Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
@jsoref
Copy link
Contributor Author

jsoref commented Oct 14, 2021

Thinking out loud, I could split the .go files by package, and then split out only the comment things, probably coalescing the comment things for packages that have a handful of changes.

Broken out by package
% for a in $(echo $packages); do echo $a $(git diff --stat master -- $a |tail -1); done | sort -n -k2
pkg/bench/ 1 file changed, 1 insertion(+), 1 deletion(-)
pkg/build/ 1 file changed, 1 insertion(+), 1 deletion(-)
pkg/keys/ 1 file changed, 1 insertion(+), 1 deletion(-)
pkg/migration/ 1 file changed, 1 insertion(+), 1 deletion(-)
pkg/multitenant/ 1 file changed, 1 insertion(+), 1 deletion(-)
pkg/settings/ 1 file changed, 4 insertions(+), 4 deletions(-)
pkg/startupmigrations/ 1 file changed, 2 insertions(+), 2 deletions(-)
pkg/acceptance/ 2 files changed, 2 insertions(+), 2 deletions(-)
pkg/base/ 2 files changed, 2 insertions(+), 2 deletions(-)
pkg/blobs/ 2 files changed, 2 insertions(+), 2 deletions(-)
pkg/config/ 2 files changed, 2 insertions(+), 2 deletions(-)
pkg/jobs/ 4 files changed, 13 insertions(+), 13 deletions(-)
pkg/rpc/ 4 files changed, 4 insertions(+), 4 deletions(-)
pkg/cloud/ 5 files changed, 8 insertions(+), 8 deletions(-)
pkg/col/ 6 files changed, 10 insertions(+), 10 deletions(-)
pkg/internal/ 6 files changed, 7 insertions(+), 7 deletions(-)
pkg/security/ 7 files changed, 10 insertions(+), 10 deletions(-)
pkg/testutils/ 7 files changed, 7 insertions(+), 7 deletions(-)
pkg/ts/ 9 files changed, 24 insertions(+), 24 deletions(-)
pkg/roachpb/ 10 files changed, 17 insertions(+), 17 deletions(-)
pkg/storage/ 10 files changed, 18 insertions(+), 18 deletions(-)
pkg/workload/ 12 files changed, 20 insertions(+), 20 deletions(-)
pkg/geo/ 13 files changed, 15 insertions(+), 15 deletions(-)
pkg/server/ 17 files changed, 28 insertions(+), 28 deletions(-)
pkg/cli/ 18 files changed, 24 insertions(+), 24 deletions(-)
pkg/cmd/ 33 files changed, 66 insertions(+), 66 deletions(-)
pkg/ccl/ 46 files changed, 90 insertions(+), 90 deletions(-)
pkg/util/ 51 files changed, 110 insertions(+), 110 deletions(-)
pkg/kv/ 85 files changed, 148 insertions(+), 148 deletions(-)
pkg/sql/ 130 files changed, 224 insertions(+), 224 deletions(-)

I imagine it'll take me quite a while to do this (it's definitely a free time task).

The vast majority of the changes are to comments

Looking at the first non-whitespace characters:

  • 545 //
  • 26 #

The rest are things that probably don't fall into the first two categories. My general approach is to work through the earlier categories and then rebase the remainder and see if there's anything that interests anyone. I don't expect to reach this point for a while.

  • 29 " (mix of human readable messages, class names under org.hibernate.test.annotations.access. from pkg/cmd/roachtest/tests/hibernate_blocklist.go which is almost certainly a no-go, and file names)
  • 10 t.Fatalf (1 t.Fatal()
  • 7 t.Errorf
  • 7 Title:
  • 7 name: (1 Name:)
  • 28 (everything else)

craig bot pushed a commit that referenced this pull request Oct 15, 2021
71555: text: correct spelling mistakes r=irfansharif a=jsoref

This goal of this PR is to be limited to just text files as suggested in:
#71464 (comment)

I believe these are the only covered files:
* .md
* no extension text
* .opt
* .yaml

I'm happy to drop things

* I try to avoid squashing until near the end.
* I will try to annotate the interesting bits (please bare with me, this will take a while)

Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
craig bot pushed a commit that referenced this pull request Oct 15, 2021
71559: html/style: correct spelling mistakes r=irfansharif a=jsoref

This goal of this PR is to be limited to just text files as suggested in:
#71464 (comment)

Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
craig bot pushed a commit that referenced this pull request Oct 25, 2021
70224: server: share the table cache among stores r=bananabrick a=bananabrick

Possible benefits of sharing the table cache are outlined
here: cockroachdb/pebble#1178.

This PR creates a table cache which can be shared among many
stores.

Release note: None

70239: sql: remove the interleaved logic from fetchers r=yuzefovich a=yuzefovich

**row: simplify fetcher to only handle a single table**

Previously, `row.Fetcher` could handle multiple tables at the same time.
This was needed to support the interleaved tables efficiently, but since
the interleaved tables are no more, we can simplify the fetcher now.

There are more things that can be removed from the fetcher about
handling of the interleaved tables, but I chose to defer them to a later
commit. This commit's main focus is simplifying the fetcher to work
under the assumption that it will be fetching only from a single table.

Addresses: #69150

Release note: None

**colbuilder: remove the check for index join on the interleaved index**

Interleaved indexes are no more.

Release note: None

**sql: remove the interleaved logic from fetchers**

This commit removes the remaining bits of handling the interleaved
tables from both row fetcher and cFetcher. It also removes a test that
verifies that we can read from the interleaved table restored from
a backup.

Release note: None

71642: misc: correct spelling mistakes r=irfansharif a=jsoref

This goal of this PR is to be limited to just code comments as suggested in:
#71464 (comment)

71653: tree: fix ambiguity with enum overloads r=rafiss a=otan

Resolves #71446 

Release note (sql change): Previously, certain enum builtins or
operators required an explicit enum cast. This has been reduced in some
cases.

71680: cli: add \statement-diag SQL shell command r=RaduBerinde a=RaduBerinde

This PR adds a `\statement-diag` command to the SQL shell used to list and download bundles, and reenables `EXPLAIN ANALYZE (DEBUG)` for tenants.

#### cli: extract statement diagnostics code

This commit extracts the part of the statment-diag code that interacts
with the system tables and moves it to sqlclientconn.

Release note: None

#### cli: improve statement-diag download

Minor improvement to `statement-diag download`: make the filename
optional, for consistency with the SQL shell equivalent.

Release note: None

#### cli: simplify invalid syntax paths in SQL shell

This change simplifies most invalid syntax paths while improving the
error message in a few cases.

Release note: None

#### cli: add \statement-diag SQL shell command

This commit adds a `\statement-diag` command to the SQL shell which
can be used to list and download statement bundles directly from the
shell.

This command should be useful for multitenant (where the possibilities
of downloading the bundle are more limited), as well as running demo
with a `make buildshort` binary.

The output of `EXPLAIN ANALYZE (DEBUG)` includes the exact syntax.

Informs #70931.

Release note (cli change): the SQL shell now supports a
`\statement-diag` command for listing and downloading statement
diagnostics bundles.

#### sql: enable EXPLAIN ANALYZE (DEBUG) for tenants

EXPLAIN ANALYZE (DEBUG) was disabled for tenants because the returned URL
was unusable.

We can now download bundles through the SQL shell, so we can reenable
EXPLAIN ANALYZE (DEBUG). For non-system tenants, we hide the URL and
only show the SQL shell and cli options.

As part of this change, we also add tenant support to the interactive
cli test infrastructure and we implement the `--background` and
`--pid-file` flags for `mt start-sql`.

Fixes #70931.

Release note (sql change): EXPLAIN ANALYZE (DEBUG) can now be used by
tenants.

Co-authored-by: Arjun Nair <nair@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-community Originated from the community X-blathers-untriaged blathers was unable to find an owner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants