-
Notifications
You must be signed in to change notification settings - Fork 36
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
chore: add multi provider appendix #264
chore: add multi provider appendix #264
Conversation
77eba85
to
8175df3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please put each sentence on its own line? it makes reviews a bit easier.
8175df3
to
bfe1fec
Compare
I would recommend we add this to included-utilities, since to me that's what this is. @beeme1mr @Kavindu-Dodan @lukas-reining @thomaspoignant what do you guys think? Should this be separate? |
bfe1fec
to
02d4e75
Compare
Yes, I also thought about this in the initial discussion @toddbaert. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me, I would just prefer to have this as a subpage or something else of Appendix A: Included Utilities
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really well written 🙌
I would recommend we add this to included-utilities, since to me that's what this is.
@toddbaert Given the length and the amount of details of this section, I think this deserves its own section. We could cross link from appendix-a to this (appendix-d) with a short description, similar to Appendix B: Gherkin Suites
I think it's fine just to have it as another heading. We can add some collapsible sections if we want. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve in spirit, but I would like to see this in the included utils. I found a few small nits as well.
Overall very well-written and clear, nice work.
4560a1e
to
11bd265
Compare
@toddbaert I moved it to the appendix A section, how's it look in there? |
@ajwootto looks great! I can't edit your branch, so I've made a small PR which fixes the lint errors: DevCycleHQ-Sandbox#1. Please merge this into your branch and I'll then merge this PR. |
Signed-off-by: Adam Wootton <[email protected]>
Signed-off-by: Adam Wootton <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
234b698
to
99655fb
Compare
Thanks! |
This PR