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

DOCSP-32693: FindOne by ObjectId #297

Merged
merged 17 commits into from
Oct 5, 2023

Conversation

norareidy
Copy link
Collaborator

@norareidy norareidy commented Sep 29, 2023

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-32693
Staging - https://docs-mongodbcom-staging.corp.mongodb.com/golang/docsworker-xlarge/DOCSP-32693-find-by-id/fundamentals/crud/read-operations/retrieve/

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?

@norareidy norareidy marked this pull request as ready for review September 29, 2023 20:37
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
@norareidy norareidy requested a review from rustagir October 2, 2023 20:16
Copy link
Collaborator

@rustagir rustagir left a comment

Choose a reason for hiding this comment

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

a few more things - lmk if you have questions about the struct comment

source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
@rustagir rustagir self-requested a review October 3, 2023 18:56
Copy link
Collaborator

@rustagir rustagir left a comment

Choose a reason for hiding this comment

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

lgtm with two small items! Great job, I know the code with going back and forth b/t bson and go is hard :)

:language: none
:visible: false

{"item":"Hibiscus","rating":4}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: this output shows the field names as lowercase, but if the code is unmarshalling the document to the Review type, it should look different, as it should print with the fieldnames from the struct.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When I run the code, it outputs the lowercase field names so maybe it's not unmarshalling to a Review? I might be missing something in the code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the field names will stay lowercase after using the MarshalExtJSON() function, unless I unmarshal again into the struct. Looks like the printed documents on this page also have lowercase fields.

source/fundamentals/crud/read-operations/retrieve.txt Outdated Show resolved Hide resolved
@matthewdale matthewdale self-requested a review October 4, 2023 18:52
Copy link
Collaborator

@matthewdale matthewdale 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 👍

@norareidy norareidy merged commit b488097 into mongodb:master Oct 5, 2023
2 checks passed
@norareidy norareidy deleted the DOCSP-32693-find-by-id branch October 5, 2023 16:43
norareidy added a commit that referenced this pull request Oct 5, 2023
* DOCSP-32693: FindOne by ObjectId

* fixing code

* adding callout + text

* code spacing

* reword

* error message

* most feedback + stage

* adding label

* fix highlight

* move code to separate file

* fix literalinclude

* includes

* io code blocks

* feedback pt 2

* code typos

* last suggestions

* fix

(cherry picked from commit b488097)
norareidy added a commit that referenced this pull request Oct 5, 2023
* DOCSP-32693: FindOne by ObjectId

* fixing code

* adding callout + text

* code spacing

* reword

* error message

* most feedback + stage

* adding label

* fix highlight

* move code to separate file

* fix literalinclude

* includes

* io code blocks

* feedback pt 2

* code typos

* last suggestions

* fix

(cherry picked from commit b488097)
(cherry picked from commit 36da935)
norareidy added a commit that referenced this pull request Oct 5, 2023
* DOCSP-32693: FindOne by ObjectId

* fixing code

* adding callout + text

* code spacing

* reword

* error message

* most feedback + stage

* adding label

* fix highlight

* move code to separate file

* fix literalinclude

* includes

* io code blocks

* feedback pt 2

* code typos

* last suggestions

* fix

(cherry picked from commit b488097)
(cherry picked from commit 36da935)
norareidy added a commit that referenced this pull request Oct 5, 2023
* DOCSP-32693: FindOne by ObjectId

* fixing code

* adding callout + text

* code spacing

* reword

* error message

* most feedback + stage

* adding label

* fix highlight

* move code to separate file

* fix literalinclude

* includes

* io code blocks

* feedback pt 2

* code typos

* last suggestions

* fix

(cherry picked from commit b488097)
(cherry picked from commit 36da935)
norareidy added a commit that referenced this pull request Oct 5, 2023
* DOCSP-32693: FindOne by ObjectId

* fixing code

* adding callout + text

* code spacing

* reword

* error message

* most feedback + stage

* adding label

* fix highlight

* move code to separate file

* fix literalinclude

* includes

* io code blocks

* feedback pt 2

* code typos

* last suggestions

* fix

(cherry picked from commit b488097)
(cherry picked from commit 36da935)
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