Skip to content
This repository has been archived by the owner on Dec 14, 2017. It is now read-only.

Support sub-resources in discovery API generation #234

Closed
wants to merge 5 commits into from

Conversation

dgresham
Copy link

This prefixes resource names with the name of their parent resource, which can lead to some unwieldy Resource and Request class names; it's an easier way to disambiguate than splitting into more libraries still (which would be a substantial refactor, I believe).

emitter_test.dart doesn't seem to be run by the presubmit script, and I haven't yet got it to run myself, but I believe it should work (I've tested the apigen binary manually, and it looks correct).

Douglas Gresham added 3 commits August 21, 2014 12:36

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
…, and not in the requests file)

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@yjbanov
Copy link
Member

yjbanov commented Aug 22, 2014

Our tests for the code generator, which includes the emitter, are end-to-end, testing the output of the generator given sample input. These tests are under test/generated. To add a test, create 3 files:

PREFIX_test.dart - the actual unit test code
PREFIX_test.json - sample input discovery schema for the generator
PREFIX_test.streamy.yaml - configuration for the generator

Streamy's pub transformer will automatically pick up the new files and run them through the generator, making the output available to the test.

Add the new test to test/all_tests.dart. Finally, to run tests:

$ pub serve
# Then in another terminal window
$ dart -c --package-root=http://localhost:8081/packages http://localhost:8081/all_tests.dart

BTW, heads up, we're going to be promoting master-v0.2 to master today or tomorrow after #233 is in, so you might need to rebase and create a new PR (unfortunately Github doesn't support changing target branch for existing PR).

Douglas Gresham added 2 commits August 22, 2014 13:21

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
…rces (which was a bug caught by said tests)

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
…sub-resources are concatenated with the names of their parent resources (to avoid naming conflicts)
@dgresham
Copy link
Author

OK - I can do that, no problem. Have added tests (and they uncovered a bug, yay!). Will keep an eye out for the branch change and re-send.

@yjbanov yjbanov closed this Aug 23, 2014
@yjbanov
Copy link
Member

yjbanov commented Aug 24, 2014

master-v0.2 branch is now master. Please resend your PR onto master (also, fetch and rebase; a small fix in the emitter went in recently). Looked briefly at the code changes. They look great so far. Will dive a little deeper tomorrow.

@dgresham dgresham deleted the sub-resources-v0.2 branch August 25, 2014 14:08
@dgresham
Copy link
Author

Done - #237

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants