Add support for generating android matchers #425
Conversation
DanielMSchmidt
force-pushed
the
generation/java-support
branch
3 times, most recently
from
b5e1218 to
69ccd0f
Compare
|
Awesome, the build is green again, thanks to rebasing <3 |
| @@ -0,0 1,142 @@ | |||
| /** | |||
There was a problem hiding this comment.
This is unrelated, probably needs to be removed in the generation code
There was a problem hiding this comment.
This will be a seperate PR. The generation is currently "dump" and nothing keeps track if these functions are needed or not. It might be a little more complex and unrelated to this set of changes to remove these in the generation. But it's one of my next ups on the list
| } | ||
| } | ||
|
|
||
| function sanitize_uiAccessibilityTraits(value) { |
| const ast = t.program([createClass(json), createExport(json)]); | ||
| const output = generate(ast); | ||
|
|
||
| const commentBefore = "/**\n\n\tThis code is generated.\n\tFor more information see generation/README.md.\n*/\n\n"; |
There was a problem hiding this comment.
Maybe add a more visible comment, and warning, to make sure people won't try adding things manually.
something like
// This file is automatically generated.
// Do not modify this file -- YOUR CHANGES WILL BE ERASED!There was a problem hiding this comment.
Yeah, sounds nice, will do this in a seperate small PR, to keep this change contained 👍 It's on my list now
| @@ -1,5 1,6 @@ | |||
| const invoke = require('../invoke'); | |||
There was a problem hiding this comment.
Don't you want to add the other actions as well ?
Did you run the Android E2E suite with the new implementation ?
There was a problem hiding this comment.
I ran the e2e suite with the old implementation, will re-run it the next days with the newer one. As I said, I would like to add the actions step by step to have smaller, easier to merge PRs.
|
Thank your for the feedback @rotemmiz - I added cards for the smaller separated tasks in our project board, see here and here. If I understand you correctly there are no blockers for merging here, so I would run a last test with the current test setup beginning this weekend and merge the stuff in if everything goes green. Separate PRs for the issues above are incoming hopefully this weekend, too. |
This restructures the generation to allow for multiple adapters, leaving this open for further extension. It only adds one generated action to the live code, so that the diff doesn't get any bigger
DanielMSchmidt
force-pushed
the
generation/java-support
branch
from
69ccd0f to
a5c338d
Compare
|
@rotemmiz Works on my machine, if I hear no veto, I will merge it tomorrow 👍 |
This restructures the generation to allow for multiple adapters, leaving this open for further extension. It only adds one generated action to the live code, so that the diff doesn't get any bigger.
Although it might look intimidating as a diff, most of it is just moving files around and extracting the core generation from the platform specifics.