-
Notifications
You must be signed in to change notification settings - Fork 2
docs(ci-services): initial pass at detailed docs #648
base: master
Are you sure you want to change the base?
Conversation
just a first pass, but can at least give us something to talk about. do we have the right balance of prose vs. technical docs, etc. also, i'd love to create valid links for each of the more general items i reference. i think each one of them can be confusing without a little context. might be best to remove them prior to merging this if we decide not to do that yet, though.
Codecov Report
@@ Coverage Diff @@
## master #648 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 44 44
Lines 270 270
=========================================
Hits 270 270 Continue to review full report at Codecov.
|
thanks for getting this kicked off. this seems like it is headed in a good direction my initial comments are just from a quick pass and mostly thinking out loud to just get the convo going. the one other piece that i think might be valuable could be to touch on what can be consistently expected back as result data. do you think that makes sense to fit in here? |
i also think it would be good to link to https://github.com/form8ion/awesome#continuous-integration-services |
docs/ci-services.md
Outdated
values are: `Public`, `Private`) | ||
* `projectType` __string__ type of project to be scaffolded (valid values are: | ||
`Application`, `Package`, `CLI`) | ||
* `nodeVersionCategory` __string__ node category chosen for the project (valid |
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.
* `nodeVersionCategory` __string__ node category chosen for the project (valid | |
* `nodeVersionCategory` __string__ category of node version chosen for the project (valid |
docs/ci-services.md
Outdated
* `unit` __boolean__ whether the project will be unit-tested | ||
* `integration` __boolean__ whether the project will be integration-tested | ||
|
||
*Response* |
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 typically call this results. not sure response is a good fit since that makes me think request/response, which i dont think this really is
docs/ci-services.md
Outdated
The response object is commonly used across the project as a way for scaffolders | ||
to communicate information to the parent. | ||
|
||
* `nextSteps` __array__ list of strings that will be aggregated and displayed |
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.
* `nextSteps` __array__ list of strings that will be aggregated and displayed | |
* `nextSteps` __array__ list of objects with shape of {summary: __string__, description: __markdown string__} that will be aggregated and displayed |
not quite a suggestion of exact content, but this is the more correct type
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 got this shape from here. am i understanding that one wrong?
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.
yeah, which might be pointing out confusing api design. the important piece to consider here is order of operations and apparently that multiple returned objects are using the same attribute name.
the results from sub-scaffolders, like the ci plugins, have this shape, which are processed in that case to create github issues for each one. the version that you linked to is a list of urls that point to the created issues. keep in mind that the subscaffolders dont know which processors will read the results and that the github one is only one processor
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.
nice, good to know 👍 . i went ahead and made the change, but i'm still confused. the docs we're shooting for in this change are the return values from the ci scaffolder. from what i'm seeing in the example you linked above it looks like the return value from the github scaffolder contains an object with nextSteps
that is just a list of urls.
this is making me think you might be talking about the values passed in to the ci scaffolder, which i don't even have in the list of arguments to the top-level scaffolder
on the ci-services object.
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.
oh, right... yeah, we probably need to improve the naming even more if i'm getting confused by this. you're right that i was getting the input/output wrong, but the reason for me getting it wrong is that i was pointing out the version of nextSteps
that is more consistently returned from general plugins. i was still thinking from the perspective of working toward documenting the general case and calling out how the specific cases differ from that. even calling out this difference is really confusing when they use the same name for very different purposes
just a first pass, but can at least give us something to talk about. do we have
the right balance of prose vs. technical docs, should we even mention
"user selects in the CLI", etc. also, i'd love to create
valid links for each of the more general items i reference. i think each one
of them can be confusing without a little context. might be best to remove them
prior to merging this if we decide not to do that yet, though.