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.

Sign up for GitHub

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

Jump to bottom

Correct some typos/mistakes in comments/docs #88446

Merged
merged 2 commits into from
Jul 12, 2022
Merged

Correct some typos/mistakes in comments/docs #88446

merged 2 commits into from
Jul 12, 2022

Conversation

pxsalehi
Copy link
Member

@pxsalehi pxsalehi commented Jul 11, 2022 •
edited
Loading

No description provided.

@pxsalehi pxsalehi added >non-issue :Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. labels Jul 11, 2022
@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Jul 11, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

pxsalehi
pxsalehi commented Jul 11, 2022
View reviewed changes
server/src/main/java/org/elasticsearch/snapshots/package-info.java
@@ -43,9 +43,9 @@
* {@code UpdateIndexShardSnapshotStatusRequest}.</li>
*
* <li>If as a result of the received status update requests, all shards in the cluster state are in a completed state, i.e are marked as
* either {@code SUCCESS}, {@code FAILED} or {@code MISSING}, the {@code SnapshotShardsService} will update the state of the {@code Entry}
* either {@code SUCCESS}, {@code FAILED} or {@code MISSING}, the {@code SnapshotsService} will update the state of the {@code Entry}
Copy link
Member Author

Choose a reason for hiding this comment

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

I believe this was a mistake. @original-brownbear, could you please confirm that?

Copy link
Member

Choose a reason for hiding this comment

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

++ right this changed eventually and the docs here became outdated.

original-brownbear
original-brownbear reviewed Jul 11, 2022
View reviewed changes
Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

Looks good with one exception. Thanks @pxsalehi !

server/src/main/java/org/elasticsearch/snapshots/package-info.java Outdated
* writing the metadata necessary to finalize the snapshot in the repository to the repository.</li>
* writing to the repository the metadata necessary to finalize the snapshot.</li>
Copy link
Member

Choose a reason for hiding this comment

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

This sentence was intentional duplication/repetition actually. This was to convey that the snapshot is finalized as far as files/blobs in the repository are concerned.
If we want to change this, it'd be nice to retain that meaning.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see! I added it back but not in the same place! Please see e90e51e.

server/src/main/java/org/elasticsearch/snapshots/package-info.java
@@ -43,9 +43,9 @@
* {@code UpdateIndexShardSnapshotStatusRequest}.</li>
*
* <li>If as a result of the received status update requests, all shards in the cluster state are in a completed state, i.e are marked as
* either {@code SUCCESS}, {@code FAILED} or {@code MISSING}, the {@code SnapshotShardsService} will update the state of the {@code Entry}
* either {@code SUCCESS}, {@code FAILED} or {@code MISSING}, the {@code SnapshotsService} will update the state of the {@code Entry}
Copy link
Member

Choose a reason for hiding this comment

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

++ right this changed eventually and the docs here became outdated.

@pxsalehi
Copy link
Member Author

Thank you, Armin and Ievgen!

@pxsalehi pxsalehi merged commit a4ec9c6 into elastic:master Jul 12, 2022
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Jul 12, 2022
@weizijun
Merge branch 'upstream/master' into rollup-improve-logger
b9e97db 
* upstream/master:
  Pass IndexMetadata to AllocationDecider.can_remain (elastic#88453)
  [TSDB] Cache rollup bucket timestamp to reduce rounding cost (elastic#88420)
  Correct some typos/mistakes in comments/docs (elastic#88446)
  Make ClusterInfo use immutable maps in all cases (elastic#88447)
  Reduce map lookups (elastic#88418)
  Don't index geo_shape field in AbstractBuilderTestCase (elastic#88437)
  Remove usages of TestGeoShapeFieldMapperPlugin from enrich module (elastic#88440)
  Fix test memory leak (elastic#88362)
  Improve error when sorting on incompatible types (elastic#88399)
  Remove usages of BucketCollector#getLeafCollector(LeafReaderContext) (elastic#88414)
  Mute ReactiveStorageIT::testScaleWhileShrinking (elastic#88431)
  Clarify snapshot docs on archive indices (elastic#88417)
  [Stack Monitoring] Switch cgroup memory fields to keyword (elastic#88260)
  Fix RealmIdentifier XContent parser (elastic#88410)
  Make LoggedExec gradle task configuration cache compatible (elastic#87621)
  Update CorruptedFileIT so that it passes with new allocation strategy (elastic#88314)
  Update RareClusterStateIT to work with the new shards allocator (elastic#87922)
  Ensure CreateApiKey always creates a new document (elastic#88413)

# Conflicts:
#	x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/RollupShardIndexer.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@original-brownbear original-brownbear original-brownbear approved these changes

@idegtiarenko idegtiarenko idegtiarenko approved these changes

Assignees
No one assigned
Labels
:Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >non-issue Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.4.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pxsalehi @elasticmachine @idegtiarenko @original-brownbear @elasticsearchmachine