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

Document why we add '$' prefix to result local in constructors #870

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

osa1
Copy link
Member

@osa1 osa1 commented Aug 15, 2023

We used to add a prefix before, but when I brought back the constructor
arguments I didn't include it because (1) the previous prefix was _ which
caused lint errors (2) there were no tests or documentation on why the prefix
was needed.

Document it now so that it won't be removed again.

(I couldn't add this in #869 as I didn't have push access to the PR's repo)

@osa1 osa1 requested a review from sigurdm August 15, 2023 06:50
@osa1 osa1 merged commit 41d0156 into google:master Aug 15, 2023
17 of 18 checks passed
@osa1 osa1 deleted the document_result_field branch August 15, 2023 08:55
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.

2 participants