[RFC][DO_NOT_MERGE] Kpact #3
Conversation
Including tested code examples and open design decisions
|
Documentation is available at https://usr42.github.io/pact-jvm/ |
There was a problem hiding this comment.
In general, I like this.
You could consider another use case, which i don't know if it makes sense at all, but here goes:
People might want to use your API in unexpected ways. E.g. extracting parts of the definition into variables and reusing them multiple times in different contexts.
I'm not sure, if the current code supports this, since I've seen only 2 immutable objects.
pact-jvm-consumer-kotlin/README.adoc
Outdated
| [source,java,indent=0,role="primary"] | ||
| .Infix DSL | ||
| ---- | ||
| include::src/test/kotlin/au/com/dius/pact/consumer/kotlin/dsl/examples/InfixDslExample.kt[lines=11..47] |
There was a problem hiding this comment.
Github doesn't render this properly, which results in an unexpected experience. I expect the README to render correctly and have the complete content available. Having a short readme and linking to the github.io page is also fine.
There was a problem hiding this comment.
done
I've split into a brief README.md and a detailed, parsed asciidoc-generated gh-page linked in the readme.
pact-jvm-consumer-kotlin/README.adoc
Outdated
| [source,java,indent=0,role="secondary"] | ||
| .Fluent DSL | ||
| ---- | ||
| include::src/test/kotlin/au/com/dius/pact/consumer/kotlin/dsl/examples/FluentDslExample.kt[lines=11..47] |
There was a problem hiding this comment.
Referencing line numbers is probably fragile. Consider using tags instead
There was a problem hiding this comment.
I wasn't aware of tags for includes. Awesome feature. Thanks for the hint.
done
pact-jvm-consumer-kotlin/README.adoc
Outdated
| :n: icon:times[role="red"] | ||
|
|
||
| == Goals | ||
| * Easy readable |
There was a problem hiding this comment.
The prose could be more extensive here, e.g. the resulting pact should be readable. Also should they be easy or simple https://www.youtube.com/watch?v=34_L7t7fD_U
There was a problem hiding this comment.
done
I reformulated the first two bullet points
pact-jvm-consumer-kotlin/README.adoc
Outdated
| * Things on the same logical level are on the same indentation level (auto-indentation of IDE) | ||
| * Usable also from https://github.com/DiUS/pact-jvm/tree/master/pact-jvm-consumer-junit5[pact-jvm-consumer-junit5] | ||
| (from `PactDslWithProvider` to `RequestResponsePact`) | ||
| * Support both: |
pact-jvm-consumer-kotlin/README.adoc
Outdated
| [source,java,indent=0,role="primary"] | ||
| .infix | ||
| ---- | ||
| consumer withName "consumer" andProvider "provider" havePact { |
| @@ -0,0 +1,79 @@ | |||
| plugins { | |||
| id "io.gitlab.arturbosch.detekt" version "1.0.0.RC8" | |||
| id "com.github.ben-manes.versions" version "0.20.0" | |||
There was a problem hiding this comment.
Consider using https://github.com/patrikerdes/gradle-use-latest-versions-plugin too
pact-jvm-consumer-kotlin/README.adoc
Outdated
| [source,java,indent=0,role="secondary"] | ||
| .fluent | ||
| ---- | ||
| "consumer".hasPactWith("provider") { |
There was a problem hiding this comment.
I feel like a string extension goes too far. A string does not represent a consumer by itself. So "consumer".hasPactWith reads kind of non-sensical to me
There was a problem hiding this comment.
This String extension is only applicable inside of the kPact DSL lambda. So, it is not a general String extension.
pact-jvm-consumer-kotlin/README.adoc
Outdated
| "consumer".hasPactWith("provider") { | ||
| ---- | ||
|
|
||
| ==== (Dis-)Advantages |
There was a problem hiding this comment.
Consider a title like: Comparing the different DSL Styles
pact-jvm-consumer-kotlin/README.adoc
Outdated
| // ... | ||
| } | ||
|
|
||
| consumer withName "consumer" andProvider "provider" havePact { |
There was a problem hiding this comment.
I'm not sure I understand your concern here. Couldn't this second call raise an exception, due to a conflict with an existing pact?
There was a problem hiding this comment.
The problem is the missing compile time safety. The exception is thrown during runtime, so the compiler and the IDE cannot help to prevent this error. With the other approach this error is not possible at all.
| fun infixDsl() { | ||
| val infixDsl = | ||
| // tag::CamelCase[] | ||
| KPact between "Some Consumer" and "Some Provider" isDefinedBy { // <1> <2> |
There was a problem hiding this comment.
I prefer this one. One word usually makes it easier to use in an IDE. I personally like it to be a little more explicit, thus I prefer this over the very short option.
| fun proseLikeShorter() { | ||
| val infixDsl = | ||
| // tag::proseLikeShorter[] | ||
| KPact between "Some Consumer" and "Some Provider" { // <1> <2> |
There was a problem hiding this comment.
I prefer this one, it is more concise
This PR is an RFC and only for discussing the current state of the DSL and how it should evolve.
It's not intended to be merged.