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

docs(ci-services): initial pass at detailed docs #648

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

trevtrich
Copy link
Member

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.

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
Copy link

codecov bot commented Dec 10, 2020

Codecov Report

Merging #648 (21c7837) into master (9f7862e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f7862e...21c7837. Read the comment docs.

docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Show resolved Hide resolved
@travi
Copy link
Member

travi commented Dec 10, 2020

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?

@travi
Copy link
Member

travi commented Dec 11, 2020

i also think it would be good to link to https://github.com/form8ion/awesome#continuous-integration-services

docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `nodeVersionCategory` __string__ node category chosen for the project (valid
* `nodeVersionCategory` __string__ category of node version chosen for the project (valid

* `unit` __boolean__ whether the project will be unit-tested
* `integration` __boolean__ whether the project will be integration-tested

*Response*
Copy link
Member

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

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `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

Copy link
Member Author

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?

Copy link
Member

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

Copy link
Member Author

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.

Copy link
Member

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

docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
docs/ci-services.md Outdated Show resolved Hide resolved
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.

2 participants