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

chore(gae): migrate region tags from projectionqueries.go #5108

Merged
merged 5 commits into from
Jan 30, 2025
Merged

chore(gae): migrate region tags from projectionqueries.go #5108

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1669a27
chore(gae): migrate region tags from projectionqueries.go
OremGLG Jan 24, 2025
0d9a4b6
Merge branch 'main' into garciadiaz-chore-migrate-region-projection-q…
telpirion Jan 28, 2025
d0a66bd
Merge branch 'main' into garciadiaz-chore-migrate-region-projection-q…
OremGLG Jan 29, 2025
f827d0d
Merge branch 'main' into garciadiaz-chore-migrate-region-projection-q…
telpirion Jan 30, 2025
db4c14f
Merge branch 'main' into garciadiaz-chore-migrate-region-projection-q…
telpirion Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/appengine/datastore/projectionqueries/projectionqueries.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,16 @@ type EventLog struct {
}

func example() {
// [START gae_datastore_using_1]
// [START using_1]
q := datastore.NewQuery("People").Project("FirstName", "LastName")
// [END using_1]
// [END gae_datastore_using_1]
Comment on lines +31 to +35
Copy link

Choose a reason for hiding this comment

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

low

Can we consolidate the start and end tags for gae_datastore_using_1 and using_1 into a single tag? This would reduce redundancy and improve readability. For example:

// [START gae_datastore_using_1]
q := datastore.NewQuery("People").Project("FirstName", "LastName")
// [END gae_datastore_using_1]
Suggested change
// [START gae_datastore_using_1]
// [START using_1]
q := datastore.NewQuery("People").Project("FirstName", "LastName")
// [END using_1]
// [END gae_datastore_using_1]
// [START gae_datastore_using_1]
q := datastore.NewQuery("People").Project("FirstName", "LastName")
// [END gae_datastore_using_1]

_ = q
}

func example2() {
// [START gae_datastore_using_2]
// [START using_2]
q := datastore.NewQuery("EventLog").
Project("Title", "ReadPath", "DateWritten").
Expand All @@ -53,31 +56,38 @@ func example2() {
log.Infof(ctx, "Log record: %v, %v, %v", l.Title, l.ReadPath, l.DateWritten)
}
// [END using_2]
// [END gae_datastore_using_2]
}

func example3() {
// [START gae_grouping_grouping]
// [START grouping]
q := datastore.NewQuery("Person").
Project("LastName", "Height").Distinct().
Filter("Height >", 20).
Order("-Height").Order("LastName")
// [END grouping]
// [END gae_grouping_grouping]
Comment on lines +63 to +70
Copy link

Choose a reason for hiding this comment

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

low

The region tags gae_grouping_grouping and grouping seem redundant. It's best to avoid redundant tags and use a single, descriptive tag. Consider using just gae_datastore_grouping to ensure consistency with other examples. For example:

// [START gae_datastore_grouping]
q := datastore.NewQuery("Person").
Project("LastName", "Height").Distinct().
Filter("Height >", 20).
Order("-Height").Order("LastName")
// [END gae_datastore_grouping]
Suggested change
// [START gae_grouping_grouping]
// [START grouping]
q := datastore.NewQuery("Person").
Project("LastName", "Height").Distinct().
Filter("Height >", 20).
Order("-Height").Order("LastName")
// [END grouping]
// [END gae_grouping_grouping]
// [START gae_datastore_grouping]
q := datastore.NewQuery("Person").
Project("LastName", "Height").Distinct().
Filter("Height >", 20).
Order("-Height").Order("LastName")
// [END gae_datastore_grouping]

_ = q

type Foo struct {
A []int
B []string
}

// [START gae_datastore_projections_and_multiple_valued_properties_1]
// [START projections_and_multiple_valued_properties_1]
entity := Foo{A: []int{1, 1, 2, 3}, B: []string{"x", "y", "x"}}
// [END projections_and_multiple_valued_properties_1]
// [END gae_datastore_projections_and_multiple_valued_properties_1]
Comment on lines +78 to +82
Copy link

Choose a reason for hiding this comment

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

low

Can we consolidate the region tags here as well, similar to the previous suggestions? Using a single tag gae_datastore_projections_and_multiple_valued_properties_1 would improve consistency and reduce redundancy. For example:

// [START gae_datastore_projections_and_multiple_valued_properties_1]
entity := Foo{A: []int{1, 1, 2, 3}, B: []string{"x", "y", "x"}}
// [END gae_datastore_projections_and_multiple_valued_properties_1]
Suggested change
// [START gae_datastore_projections_and_multiple_valued_properties_1]
// [START projections_and_multiple_valued_properties_1]
entity := Foo{A: []int{1, 1, 2, 3}, B: []string{"x", "y", "x"}}
// [END projections_and_multiple_valued_properties_1]
// [END gae_datastore_projections_and_multiple_valued_properties_1]
// [START gae_datastore_projections_and_multiple_valued_properties_1]
entity := Foo{A: []int{1, 1, 2, 3}, B: []string{"x", "y", "x"}}
// [END gae_datastore_projections_and_multiple_valued_properties_1]

_ = entity
}

func example4() {
// [START gae_datastore_projections_and_multiple_valued_properties_2]
// [START projections_and_multiple_valued_properties_2]
q := datastore.NewQuery("Foo").Project("A", "B").Filter("A <", 3)
// [END projections_and_multiple_valued_properties_2]
// [END gae_datastore_projections_and_multiple_valued_properties_2]
Comment on lines +87 to +91
Copy link

Choose a reason for hiding this comment

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

low

For consistency, let's consolidate the tags gae_datastore_projections_and_multiple_valued_properties_2 and projections_and_multiple_valued_properties_2 into a single tag gae_datastore_projections_and_multiple_valued_properties_2. For example:

// [START gae_datastore_projections_and_multiple_valued_properties_2]
q := datastore.NewQuery("Foo").Project("A", "B").Filter("A <", 3)
// [END gae_datastore_projections_and_multiple_valued_properties_2]
Suggested change
// [START gae_datastore_projections_and_multiple_valued_properties_2]
// [START projections_and_multiple_valued_properties_2]
q := datastore.NewQuery("Foo").Project("A", "B").Filter("A <", 3)
// [END projections_and_multiple_valued_properties_2]
// [END gae_datastore_projections_and_multiple_valued_properties_2]
// [START gae_datastore_projections_and_multiple_valued_properties_2]
q := datastore.NewQuery("Foo").Project("A", "B").Filter("A <", 3)
// [END gae_datastore_projections_and_multiple_valued_properties_2]

_ = q
}
Loading