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

Mark fields as readonly in docs #217

Merged
merged 3 commits into from
Aug 26, 2019
Merged

Conversation

jbolinger
Copy link
Contributor

@jbolinger jbolinger commented Aug 21, 2019

Marks the attribute as readonly in the generated docs if the OUTPUT_ONLY flag is set on the field.

I also added a field presenter test. Happy to add more tests as part of this PR, but I'm looking for advice on the structure. Should we have a folder under test/gapic/presenter/field, keep it with the method-level tests as I've done here since those presenters are fairly related, etc.? Alternatively, I can defer to #65 if you'd prefer to wait on adding these.

[resolves #103]

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 21, 2019
@jbolinger jbolinger self-assigned this Aug 21, 2019
@quartzmo
Copy link
Member

quartzmo commented Aug 21, 2019

Should we have a folder under test/gapic/presenter/field, keep it with the method-level tests as I've done here since those presenters are fairly related, etc.?

I see that we have test/gapic/presenters/method_test.rb which contains MethodPresenterTest. Personally I think the file could be renamed to presenters/method_presenter_test.rb. Your test could be added in the same directory as presenters/field_presenter_test.rb.

One note, however, is that I might have expected the presenters path to be preserved:

templates/default/helpers/presenters/method_presenter.rb

and

test/templates/default/helpers/presenters/method_presenter_test.rb

But we've done a lot of moving things around already so don't take that as a very serious recommendation, just a nit.

@jbolinger
Copy link
Contributor Author

Thanks @quartzmo - I moved the test alongside the method presenter test for consistency for now. We can always move it again later.

I also added a garbage test, but it doesn't cover all the bases yet. In particular, I couldn't figure out how to get the nested protos to pass but I think that can be addressed later.

@jbolinger jbolinger mentioned this pull request Aug 22, 2019
@quartzmo
Copy link
Member

Is anything else needed for this PR? I approved yesterday.

@blowmage Want to take a look?

@jbolinger jbolinger merged commit 9899614 into googleapis:master Aug 26, 2019
@jbolinger jbolinger deleted the readonly branch August 26, 2019 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider hiding #field= for fields marked OUTPUT_ONLY
3 participants