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

Formatting changes to templates #1667

Merged
merged 21 commits into from
Dec 12, 2024
Merged

Formatting changes to templates #1667

merged 21 commits into from
Dec 12, 2024
+2,559 −2,662

Conversation

sharan-malyala
Copy link
Contributor

@sharan-malyala sharan-malyala commented Jun 18, 2024 •
edited
Loading

Fixed formatting.

@sharan-malyala sharan-malyala marked this pull request as ready for review June 19, 2024 05:50
@sharan-malyala sharan-malyala requested a review from a team as a code owner June 19, 2024 05:50
@sharan-malyala sharan-malyala requested review from manitgupta and Deep1998 and removed request for a team June 19, 2024 05:50
@sharan-malyala sharan-malyala marked this pull request as draft June 21, 2024 04:38
@sharan-malyala sharan-malyala marked this pull request as ready for review June 21, 2024 04:39
@liferoad liferoad requested a review from rszper July 1, 2024 17:08
rszper
rszper approved these changes Jul 1, 2024
View reviewed changes
Copy link
Contributor

@rszper rszper 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 for making these updates. It's going to look much better.

v1/src/main/java/com/google/cloud/teleport/bigtable/BigtableToVectorEmbeddings.java Outdated Show resolved Hide resolved
v2/common/src/main/java/com/google/cloud/teleport/v2/options/CommonTemplateOptions.java Outdated Show resolved Hide resolved
v2/common/src/main/java/com/google/cloud/teleport/v2/options/WindowedFilenamePolicyOptions.java Outdated Show resolved Hide resolved
v2/common/src/main/java/com/google/cloud/teleport/v2/options/WindowedFilenamePolicyOptions.java Outdated Show resolved Hide resolved
...eam-to-spanner/src/main/java/com/google/cloud/teleport/v2/templates/DataStreamToSpanner.java Outdated Show resolved Hide resolved
.../main/java/com/google/cloud/teleport/v2/elasticsearch/options/ElasticsearchWriteOptions.java Outdated Show resolved Hide resolved
.../main/java/com/google/cloud/teleport/v2/elasticsearch/options/ElasticsearchWriteOptions.java Outdated Show resolved Hide resolved
.../main/java/com/google/cloud/teleport/v2/elasticsearch/options/ElasticsearchWriteOptions.java Outdated Show resolved Hide resolved
...a-common/src/main/java/com/google/cloud/teleport/v2/kafka/options/SchemaRegistryOptions.java Outdated Show resolved Hide resolved
v2/kafka-to-pubsub/src/main/java/com/google/cloud/teleport/v2/options/KafkaToPubsubOptions.java Outdated Show resolved Hide resolved
@sharan-malyala
Copy link
Contributor Author

@rszper Please confirm if these changes are good ? If yes, I will proceed to generate readme docs.

@sharan-malyala sharan-malyala requested a review from rszper July 12, 2024 04:23
@codecov Codecov
Copy link

codecov bot commented Jul 12, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.96%. Comparing base (add3dd7) to head (fc8b3aa).
Report is 18 commits behind head on main.

Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #1667      +/-   ##
============================================
+ Coverage     45.56%   45.96%   +0.39%     
- Complexity     3721     4124     +403     
============================================
  Files           847      851       +4     
  Lines         50169    50805     +636     
  Branches       5273     5343      +70     
============================================
+ Hits          22862    23351     +489     
- Misses        25632    25760     +128     
- Partials       1675     1694      +19
Components Coverage Δ
spanner-templates 67.57% <ø> (+0.56%) ⬆️
spanner-import-export 65.47% <ø> (+1.29%) ⬆️
spanner-live-forward-migration 76.30% <ø> (+0.39%) ⬆️
spanner-live-reverse-replication 77.25% <ø> (+0.38%) ⬆️
spanner-bulk-migration 86.87% <ø> (+0.30%) ⬆️
Files with missing lines Coverage Δ
...loud/teleport/plugin/model/ImageSpecParameter.java 43.91% <100.00%> (ø)
...google/cloud/teleport/bigtable/AvroToBigtable.java 62.26% <ø> (ø)
...google/cloud/teleport/bigtable/BigtableToJson.java 61.95% <ø> (ø)
...gle/cloud/teleport/bigtable/BigtableToParquet.java 37.50% <ø> (ø)
.../teleport/bigtable/BigtableToVectorEmbeddings.java 78.00% <ø> (ø)
...e/cloud/teleport/bigtable/CassandraToBigtable.java 0.00% <ø> (ø)
...gle/cloud/teleport/bigtable/ParquetToBigtable.java 57.14% <ø> (ø)
.../google/cloud/teleport/spanner/ExportPipeline.java 0.00% <ø> (ø)
...gle/cloud/teleport/spanner/TextImportPipeline.java 0.00% <ø> (ø)
...e/cloud/teleport/templates/BigQueryToTFRecord.java 19.69% <ø> (ø)
... and 27 more

... and 38 files with indirect coverage changes

@sharan-malyala sharan-malyala requested a review from a team as a code owner September 19, 2024 06:44
@sharan-malyala sharan-malyala requested a review from a team as a code owner September 19, 2024 06:44
yair-harel
yair-harel previously approved these changes Sep 19, 2024
View reviewed changes
bharadwaj-aditya
bharadwaj-aditya reviewed Sep 19, 2024
View reviewed changes
Copy link
Contributor

@bharadwaj-aditya bharadwaj-aditya left a comment

Choose a reason for hiding this comment

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

Comment for Spanner templates - Minor comment on the import pipeline. rest looks fine.

ron-gal
ron-gal reviewed Sep 19, 2024
View reviewed changes
Copy link
Contributor

@ron-gal ron-gal left a comment

Choose a reason for hiding this comment

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

Minor format comments, otherwise LGTM for bigtable changes

v2/googlecloud-to-googlecloud/README_Bigtable_Change_Streams_to_BigQuery.md Outdated Show resolved Hide resolved
...rc/main/java/com/google/cloud/teleport/v2/options/BigtableChangeStreamToBigQueryOptions.java Outdated Show resolved Hide resolved
v1/src/main/java/com/google/cloud/teleport/bigtable/AvroToBigtable.java Show resolved Hide resolved
rszper
rszper reviewed Sep 19, 2024
View reviewed changes
Copy link
Contributor

@rszper rszper left a comment

Choose a reason for hiding this comment

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

It looks like there's a lot of repetition, so the edits that I suggested should be applied to the later files as well.

v1/README_Cassandra_To_Cloud_Bigtable.md Outdated Show resolved Hide resolved
v1/README_Cloud_BigQuery_to_GCS_TensorFlow_Records.md Outdated Show resolved Hide resolved
v1/README_GCS_CSV_to_BigQuery.md Outdated Show resolved Hide resolved
v1/README_GCS_Text_to_Firestore.md Outdated Show resolved Hide resolved
v1/README_Spanner_to_GCS_Text.md Outdated Show resolved Hide resolved
v2/googlecloud-to-elasticsearch/README_GCS_to_Elasticsearch.md Outdated Show resolved Hide resolved
v2/googlecloud-to-elasticsearch/README_GCS_to_Elasticsearch_Xlang.md Outdated Show resolved Hide resolved
v2/googlecloud-to-elasticsearch/README_GCS_to_Elasticsearch_Xlang.md Outdated Show resolved Hide resolved
v2/googlecloud-to-elasticsearch/README_GCS_to_Elasticsearch_Xlang.md Outdated Show resolved Hide resolved
v2/googlecloud-to-elasticsearch/README_GCS_to_Elasticsearch_Xlang.md Outdated Show resolved Hide resolved
Polber
Polber previously approved these changes Sep 19, 2024
View reviewed changes
Copy link
Contributor

@Polber Polber left a comment

Choose a reason for hiding this comment

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

LGTM so long as other comments are addressed

v2/kafka-common/src/main/java/com/google/cloud/teleport/v2/kafka/options/KafkaReadOptions.java Outdated Show resolved Hide resolved
@sharan-malyala sharan-malyala dismissed stale reviews from Polber and yair-harel via 0daaae1 October 31, 2024 08:19
liferoad
liferoad approved these changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@liferoad liferoad left a comment

Choose a reason for hiding this comment

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

Tests are green.

@liferoad liferoad merged commit ff60a5f into GoogleCloudPlatform:main Dec 12, 2024
28 checks passed
@sharan-malyala sharan-malyala deleted the sharantej-readme branch December 12, 2024 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ron-gal ron-gal ron-gal left review comments

@rszper rszper rszper approved these changes

@bharadwaj-aditya bharadwaj-aditya bharadwaj-aditya left review comments

@liferoad liferoad liferoad approved these changes

@yair-harel yair-harel yair-harel left review comments

@Polber Polber Polber left review comments

@manitgupta manitgupta Awaiting requested review from manitgupta manitgupta is a code owner automatically assigned from GoogleCloudPlatform/spanner-migrations-team

@Deep1998 Deep1998 Awaiting requested review from Deep1998 Deep1998 is a code owner automatically assigned from GoogleCloudPlatform/spanner-migrations-team

Assignees
No one assigned
Labels
Documentation improvement size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@sharan-malyala @liferoad @Polber @bharadwaj-aditya @rszper @ron-gal @yair-harel