Skip to content

Conversation

@jsoref
Copy link
Contributor

@jsoref jsoref commented Nov 11, 2020

Looks like we're going to use this as a tracking PR for a while. I'll split off smaller sets based on recommendations and rebase this as each of merged.

Rounds:

Follow-ups:

Some thoughts:

  • This PR is too big (I know this) -- typically large projects ask about splitting in some way -- but I leave the how to the project -- it might be by top level directory, it might be by file type, it might be "comments", then "locals", then "public apis" or something else
  • I keep distinct commits by word family because it makes rebasing / dealing w/ conflicts or dropping things in case they're controversial.
  • My general preference is to squash at the very last minute (or let the project do it)
  • There are definitely some changes to java final classes -- I expect to be asked to drop these

What changes were proposed in this pull request?

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

Why are the changes needed?

Misspelled words make it harder to read / understand content.

Does this PR introduce any user-facing change?

I believe so. Unfortunately, this PR exceeds GitHub's limits, which means it'll be vaguely hard for everyone. I'll try to leave at least some marks to call some things out.

How was this patch tested?

The misspellings have been reported at jsoref@706a726#commitcomment-44064356

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

Note: 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.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@jsoref
Copy link
Contributor Author

jsoref commented Nov 11, 2020

  • I can rebase regularly or infrequently (depends on project preference).
  • I can update my changes regularly or only rarely.
    -- for large changes like this, the odds of getting conflicts are very high (which is part of why I prefer to work w/ small commits as rebuilding them individually is generally fairly easy)

@holdenk
Copy link
Contributor

holdenk commented Nov 11, 2020

How would you feel about doing this by sub project? We could start in the graph processing where it's less actively developed?

@jsoref
Copy link
Contributor Author

jsoref commented Nov 11, 2020

Fine by me

@jsoref
Copy link
Contributor Author

jsoref commented Nov 11, 2020

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Yes I like fixing typos. Breaking this out is OK too.
We don't want to break any public-facing APIs, not here, and I think you're taking good care not to.
Another concern is simply the code churn interfering with back-ports, cherry-picks, so it's OK to err on the side of not changing things, but, typo fixing is probably worth it.

srowen pushed a commit that referenced this pull request Nov 27, 2020
…rs python

### What changes were proposed in this pull request?

This PR intends to fix typos in the sub-modules:
* `R`
* `common`
* `dev`
* `mlib`
* `external`
* `project`
* `streaming`
* `resource-managers`
* `python`

Split per srowen #30323 (comment)

NOTE: The misspellings have been reported at jsoref@706a726#commitcomment-44064356

### Why are the changes needed?

Misspelled words make it harder to read / understand content.

### Does this PR introduce _any_ user-facing change?

There are various fixes to documentation, etc...

### How was this patch tested?

No testing was performed

Closes #30402 from jsoref/spelling-R_common_dev_mlib_external_project_streaming_resource-managers_python.

Authored-by: Josh Soref <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
@jsoref
Copy link
Contributor Author

jsoref commented Nov 29, 2020

Things I know will remain:

  • create (it spans multiple directories) -- we can address this after the current open PRs resolve (probably using this PR)
  • enabled (legacy_setops_precedence_enbled appears to be a public API -- addressing it would be done as its own distinct PR if at all -- one approach is to add a correct spelling making that the preferred and adding a deprecated annotation to the current spelling -- another approach is to just add a comment acknowledging the API botch)

I think that's everything, but we'll see.

Note to self: I changed one E.g. to For example (which actually fit w/ a second one that I did the same to earlier), so I'm going to have a merge conflict: resolver=drop.

maropu pushed a commit that referenced this pull request Nov 30, 2020
### What changes were proposed in this pull request?

This PR intends to fix typos in the sub-modules:
* `bin`
* `core`
* `docs`
* `external`
* `mllib`
* `repl`
* `pom.xml`

Split per srowen #30323 (comment)

NOTE: The misspellings have been reported at jsoref@706a726#commitcomment-44064356

### Why are the changes needed?

Misspelled words make it harder to read / understand content.

### Does this PR introduce _any_ user-facing change?

There are various fixes to documentation, etc...

### How was this patch tested?

No testing was performed

Closes #30530 from jsoref/spelling-bin-core-docs-external-mllib-repl.

Authored-by: Josh Soref <[email protected]>
Signed-off-by: Takeshi Yamamuro <[email protected]>
srowen pushed a commit that referenced this pull request Dec 7, 2020
### What changes were proposed in this pull request?

This PR intends to fix typos in the sub-modules:
* `sql/catalyst`
* `sql/hive-thriftserver`
* `sql/hive`

Split per srowen #30323 (comment)

NOTE: The misspellings have been reported at jsoref@706a726#commitcomment-44064356

### Why are the changes needed?

Misspelled words make it harder to read / understand content.

### Does this PR introduce _any_ user-facing change?

There are various fixes to documentation, etc...

### How was this patch tested?

No testing was performed

Closes #30532 from jsoref/spelling-sql-not-core.

Authored-by: Josh Soref <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
a0x8o added a commit to a0x8o/spark that referenced this pull request Dec 7, 2020
### What changes were proposed in this pull request?

This PR intends to fix typos in the sub-modules:
* `sql/catalyst`
* `sql/hive-thriftserver`
* `sql/hive`

Split per srowen apache/spark#30323 (comment)

NOTE: The misspellings have been reported at jsoref/spark@706a726#commitcomment-44064356

### Why are the changes needed?

Misspelled words make it harder to read / understand content.

### Does this PR introduce _any_ user-facing change?

There are various fixes to documentation, etc...

### How was this patch tested?

No testing was performed

Closes #30532 from jsoref/spelling-sql-not-core.

Authored-by: Josh Soref <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
@jsoref
Copy link
Contributor Author

jsoref commented Dec 7, 2020

Fwiw, these are the outstanding changes in this PR. I expect this PR to be closed with just one commit create. And I'll create a distinct PR for legacy_setops_precedence_enbled (enabled) and PushedFilers (filters) which, for the time being, I've added to this series.

srowen pushed a commit that referenced this pull request Dec 8, 2020
### What changes were proposed in this pull request?

This PR intends to fix typos in the sub-modules:
* `sql/core`

Split per srowen #30323 (comment)

NOTE: The misspellings have been reported at jsoref@706a726#commitcomment-44064356

### Why are the changes needed?

Misspelled words make it harder to read / understand content.

### Does this PR introduce _any_ user-facing change?

There are various fixes to documentation, etc...

### How was this patch tested?

No testing was performed

Closes #30531 from jsoref/spelling-sql-core.

Authored-by: Josh Soref <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
a0x8o added a commit to a0x8o/spark that referenced this pull request Dec 8, 2020
### What changes were proposed in this pull request?

This PR intends to fix typos in the sub-modules:
* `sql/core`

Split per srowen apache/spark#30323 (comment)

NOTE: The misspellings have been reported at jsoref/spark@706a726#commitcomment-44064356

### Why are the changes needed?

Misspelled words make it harder to read / understand content.

### Does this PR introduce _any_ user-facing change?

There are various fixes to documentation, etc...

### How was this patch tested?

No testing was performed

Closes #30531 from jsoref/spelling-sql-core.

Authored-by: Josh Soref <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
@jsoref
Copy link
Contributor Author

jsoref commented Dec 9, 2020

So, we're left with three commits:

  • 0ff10b7ee2a13df02ba2ab5c4c5c86851a531c08 - create
  • 6cbd74162a77fdc4c04c867952ec42c6e493288b - enabled
  • d0a58cc5fdb8956f41e2161a5d1f4f6d90c40353 - filters

I'm going to move enabled and filters into distinct PRs because I suspect they're more likely to be API breaks.

This PR can be closed once create (the last commit standing here) is merged.

I'm not entirely certain the other two will be merged, and even if they are, they're probably much more complicated than generic spelling fixes (as I suspect they'll require shims).

Signed-off-by: Josh Soref <[email protected]>
@jsoref
Copy link
Contributor Author

jsoref commented Dec 9, 2020

I've split out some of the things that I dropped earlier. I'm going to leave the rest alone for now (I've left the ones we might revisit as "unresolved" to make them easier to spot).

@jsoref
Copy link
Contributor Author

jsoref commented Dec 9, 2020

See comments by @srowen @cloud-fan on 0ff10b7ee2a13df02ba2ab5c4c5c86851a531c08

cloud-fan pushed a commit that referenced this pull request Dec 11, 2020
### What changes were proposed in this pull request?

Replace `legacy_setops_precedence_enbled` with `legacy_setops_precedence_enabled`

Alternatively, `legacy_setops_precedence_enabled` could be added, and `legacy_setops_precedence_enbled` retained, and if set the code could honor it and warn about the deprecated spelling.

### Why are the changes needed?

`enabled` is misspelled in `legacy_setops_precedence_enbled`

### Does this PR introduce _any_ user-facing change?

Yes.

It would break current consumers.
Examples include:
* https://www.programmersought.com/article/87752082924/
* https://github.com/fugue-project/fugue/blob/125d873c38e18b5f09b032bd01ac47a0c6739ddc/fugue_sql/_antlr/fugue_sqlLexer.py
* https://github.com/search?q=legacy_setops_precedence_enbled&type=code

### How was this patch tested?

It's been included in #30323 for a while (and is now split out here)

Closes #30677 from jsoref/spelling-enabled.

Authored-by: Josh Soref <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
dongjoon-hyun pushed a commit that referenced this pull request Dec 22, 2020
### What changes were proposed in this pull request?

This PR intends to fix typos in the sub-modules:
* `sql/core`

Split per srowen #30323 (comment)

NOTE: The misspellings have been reported at jsoref@706a726#commitcomment-44064356

### Why are the changes needed?

Misspelled words make it harder to read / understand content.

### Does this PR introduce _any_ user-facing change?

There are various fixes to documentation, etc...

### How was this patch tested?

No testing was performed

Closes #30531 from jsoref/spelling-sql-core.

Authored-by: Josh Soref <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
(cherry picked from commit a093d6f)
Signed-off-by: Dongjoon Hyun <[email protected]>
MaxGekk pushed a commit that referenced this pull request Mar 22, 2021
### What changes were proposed in this pull request?
Consistently correct the spelling of `PushedFilters`

### Why are the changes needed?
bersprockets noted that it's wrong

### Does this PR introduce _any_ user-facing change?

Technically, I think it does. Practically, neither Google nor GitHub show anyone using `pushedFilers` outside of forks (or the discussion about fixing it started at #30323 (comment))

### How was this patch tested?
None beyond CI in the previous PR

Closes #30678 from jsoref/spelling-filters.

Authored-by: Josh Soref <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
srowen pushed a commit that referenced this pull request Mar 26, 2021
### What changes were proposed in this pull request?
Deprecating `spark.launcher.childConectionTimeout` in favor of `spark.launcher.childConnectionTimeout`

### Why are the changes needed?
srowen suggested it #30323 (comment)

### How was this patch tested?
No testing. Not even compiled

Closes #30679 from jsoref/spelling-connection.

Authored-by: Josh Soref <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
@srowen srowen closed this Jul 18, 2021
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.

7 participants