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 corrections #1798

Merged
merged 45 commits into from
Mar 30, 2022
Merged

Spelling corrections #1798

merged 45 commits into from
Mar 30, 2022

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Mar 28, 2022

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

The misspellings have been reported at jsoref@e94f980#commitcomment-69669718

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

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.

jsoref added 13 commits March 27, 2022 20:59
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
@github-actions
Copy link

github-actions bot commented Mar 28, 2022

CLA Assistant Lite bot Good! All Contributors have signed the CLA.

Copy link
Contributor Author

@jsoref jsoref left a comment

Choose a reason for hiding this comment

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

Corrections mostly automatically suggested by Google Sheets.

All fault mine.

Optional: You can use [Github desktop](https://desktop.github.com/) to greatly simplify the commit and update process.
Optional: You can use [GitHub desktop](https://desktop.github.com/) to greatly simplify the commit and update process.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Brand

// CockroackDB not support 'template' arg of CREATE DATABASE
// CockroachDB not support 'template' arg of CREATE DATABASE
Copy link
Contributor Author

@jsoref jsoref Mar 28, 2022

Choose a reason for hiding this comment

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

brand

"The execution peroid of the background thread that " +
"The execution period of the background thread that " +
Copy link
Contributor Author

Choose a reason for hiding this comment

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

User facing config

GraphTraversal<Vertex, Vertex> vertexs = g.V();
GraphTraversal<Vertex, Vertex> vertices = g.V();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is 200:1 vs vertexes

# rather than mysql silently replacing each each byte of the
# rather than mysql silently replacing each byte of the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't strictly a spelling thing, but spell checkers have been complaining about doubled words for decades, so my tool now includes this as well...

Copy link
Member

Choose a reason for hiding this comment

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

nice catch for your spelling-check tool, we do need the rule

public interface Indexfiable {
public interface Indexifiable {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

API break?

Copy link
Member

Choose a reason for hiding this comment

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

@javeme maybe we should check the influence in other repository, however we need correct it before move to apache

and thanks @jsoref for reminding, is it also an automatic rule in your tool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tool recognized this as a non-word. The correction here was manual (Google Sheets guessed Index Fiable).

Picking correct words is generally left as the responsibility of a human, although for large lists, I'm able to leverage Google Sheets corrections for some portion of items.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think "Indexable" is a more appropriate word here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I should note that the places where I comment about api breaks are purely manual, it's my way of saying "I'm a human, not a robot, I do understand that changes have consequences". They also aren't necessarily exhaustive (I tend to do this changes while exhausted...).

"snowflake.datecenter_id",
"snowflake.datacenter_id",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Config break

Copy link
Member

@imbajin imbajin Mar 28, 2022

Choose a reason for hiding this comment

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

yep, although this config influence little, we should update the doc later (mark)

public static abstract class ShardSpliter<Session extends BackendSession> {
public static abstract class ShardSplitter<Session extends BackendSession> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

API change

@jsoref jsoref mentioned this pull request Mar 28, 2022
@@ -128,7 +128,7 @@ protected void formatProperties(HugeElement element,
protected void parseProperties(HugeElement element,
TableBackendEntry.Row row) {
String properties = row.column(HugeKeys.PROPERTIES);
// Query edge will wraped by a vertex, whose properties is empty
// Query edge will wrapped by a vertex, whose properties is empty
Copy link
Member

Choose a reason for hiding this comment

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

will wrapped --> wil be wrapped? (Can this be detected? lack be)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kinda. It isn't a grammar checker, but if you know someone is going to make a certain kind of mistake and can describe it with a regular expression, then, yes.

This is the set of supplemental rules that were used for this PR:
https://github.com/jsoref/hugegraph/blob/spell-check/.github/actions/spelling/line_forbidden.patterns

Only the last one is actually fancy.

But, you could do something like:

# s.b. will _be_ ...
\bwill [a-z]{3,}ed\b

Copy link
Contributor Author

@jsoref jsoref Mar 28, 2022

Choose a reason for hiding this comment

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

Note that this heuristic would fail for will speed up (bumping to {4,} would cover that case).

I haven't thought through these edges before.

In theory, there may be grammar checkers available for GitHub, although, I can't point to any as GitHub Actions.

@imbajin
Copy link
Member

imbajin commented Mar 28, 2022

recheck

Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution! Nice tool.

@@ -448,7 +448,7 @@ protected static Response createAndAssert(String path, String body,

List<Map> vertices = readList(content, "vertices", Map.class);

Map<String, String> vertextName2Ids = new HashMap<>();
Map<String, String> vertexName2Ids = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

s/vertext/vertex

@@ -49,7 +49,7 @@ public void teardown() {
}

@Test
public void testParseRepilcaWithSimpleStrategy() {
public void testParseReplicaWithSimpleStrategy() {
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Repilc/Repilca

public interface Indexfiable {
public interface Indexifiable {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think "Indexable" is a more appropriate word here

@javeme javeme changed the title Spelling Spelling corrections Mar 28, 2022
@@ -56,7 +56,7 @@ We have checked and believe the name is [suitable](https://github.com/hugegraph/

#### Relationship with Titan/Janus Graph

In the early stage of the project, we referred to the storage structure of Titan/Janus Graph, some folks thought that HugeGraph was forked from Titan/Janus. In fact, HugeGraph is not based on these projects. HugeGraph is developed completely from scratch and in the process it addressed many new challenges. Certainly, the project was inspired by Titan/Janus and we are really gratitious for such inspirations.
In the early stage of the project, we referred to the storage structure of Titan/Janus Graph, some folks thought that HugeGraph was forked from Titan/Janus. In fact, HugeGraph is not based on these projects. HugeGraph is developed completely from scratch and in the process it addressed many new challenges. Certainly, the project was inspired by Titan/Janus and we are really gratuitous for such inspirations.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should note that the downside of a spelling oriented automated fix is that one can get a correctly spelled word that one shouldn't use. This is an example.
https://github.com/hugegraph/hugegraph/pull/1797#discussion_r836326440

jsoref added 2 commits March 28, 2022 09:33
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
jsoref added 2 commits March 28, 2022 09:55
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
@jsoref jsoref force-pushed the spelling branch 3 times, most recently from 6a1755f to 09d3a9a Compare March 28, 2022 14:06
Signed-off-by: Josh Soref <[email protected]>
jsoref added 24 commits March 28, 2022 10:11
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #1798 (922d3d3) into master (b591f7d) will increase coverage by 3.96%.
The diff coverage is 79.59%.

@@             Coverage Diff              @@
##             master    #1798      +/-   ##
============================================
+ Coverage     66.91%   70.87%   +3.96%     
  Complexity      978      978              
============================================
  Files           446      446              
  Lines         37781    37781              
  Branches       5380     5380              
============================================
+ Hits          25281    26778    +1497     
+ Misses         9776     8282    -1494     
+ Partials       2724     2721       -3     
Impacted Files Coverage Δ
...va/com/baidu/hugegraph/auth/HugeAuthenticator.java 40.21% <0.00%> (ø)
...ava/com/baidu/hugegraph/define/UpdateStrategy.java 16.36% <ø> (ø)
...n/java/com/baidu/hugegraph/version/ApiVersion.java 75.00% <ø> (ø)
...egraph/backend/store/cassandra/CassandraStore.java 74.07% <ø> (ø)
.../java/com/baidu/hugegraph/auth/ResourceObject.java 96.55% <ø> (ø)
...in/java/com/baidu/hugegraph/auth/SchemaDefine.java 76.10% <ø> (ø)
...gegraph/backend/serializer/BinaryBackendEntry.java 79.54% <ø> (+5.68%) ⬆️
...ugegraph/backend/serializer/TableBackendEntry.java 88.88% <ø> (ø)
...hugegraph/backend/serializer/TextBackendEntry.java 83.33% <ø> (ø)
...u/hugegraph/backend/serializer/TextSerializer.java 86.24% <ø> (ø)
... and 150 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b591f7d...922d3d3. Read the comment docs.

Copy link
Member

@imbajin imbajin left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, u could submit another pr for github action u mentioned before ~

@javeme javeme merged commit 5368f32 into apache:master Mar 30, 2022
@jsoref jsoref deleted the spelling branch March 30, 2022 09:26
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.

3 participants