Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Document CI / Devop pipelines #1731
base: main
Are you sure you want to change the base?
Document CI / Devop pipelines #1731
Changes from all commits
05ef58d
b211da6
93d2b87
2f6bea1
0077680
f40ef64
154ef0f
b57a867
61d9a29
708a52e
e55b01d
3294835
ee8d022
6c99582
66ee19f
d9efda1
d152a90
f4517af
2f95535
f7ca008
6112532
0371a47
6124ec0
ae3444a
f20f228
66f74cd
74d8816
45ecd7b
6ff1edc
a3372b4
452382c
b26cedd
5ee0708
bdba1c2
937c4fe
11cbe26
a7e6749
b583d1d
e5fc81d
49a0015
d809661
2fd049d
ff1bca0
112178d
bd630b6
9fba827
62f8ac8
5755076
65ae4c6
8aadf99
1074b19
900535c
94a468c
ce8da83
3b2906e
ae16809
fc8b92b
f811edf
e4677ef
62be611
2f6504f
643bc08
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 file doesn't cover the topics:
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.
On this file, I think it would be useful to take a step back and not miss the forest for the trees. It does a good job at introducing some circle CI concepts (that I'd like to just be completed from my comments), but in some ways it doesn't really explain the specific navigation2 use of CircleCI; which mostly just just about the 3 workflows (what they do, when they run, why they run, their general description of steps) that is generally missing or overlooked in favor of explaining the components they're made of abstractly.
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.
The first workflow I think covers both PRs to test them and then also the main branch after merging for the new code introduction. That should be mentioned specifically.
On the third item, I thought that cron job was setup in Dockerhub? I assume we moved it over here at some point, that makes sense then your dockerhub trigger API references in the dockerhub section. Please make sure you close that loop in the dockerhub page
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.
Be specific here, why are you talking about this? You should tell the reader why how you're using the
requires
in each of the 3 use cases you highlight in the paragraph above. Clearly one is for the 4 build/test jobs for the main CI situation. You abstractly say that it "may of course be parallelized in the CI pipeline", so tell them why you're bringing this up at all - you're leveraging that feature to speed up the pipelines by having many running jobs in parallel. Specifically go over that design for the 3 cases since this is the reference guide for our CircleCI setup. "We use this in the PR workflow by.... We use this in the nightly workflow by... etc"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.
A hyperlink to the docs on "executors" would be good to be consistent with your other ones like jobs/workflows
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.
You gloss over how the build and test steps share the built workspace to test on, please go into detail about that.
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.
A new paragraph talking 1-2 sentences about the nightly jobs and dockerhub cron job would be good references to round it off
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.
"leveraged later for splitting longer tests across multiple containers for that job" where do we do that?
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.
Honestly, I'm not sure what this paragraph is trying to communicate or if it aids in explaining the concept of jobs and how we use them in Circle
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 paragraph would be benefited from explaining the why there's caching in both the docker images and in circle CI, why not just do one? Why not just have circle just build the dockerfile? (those kinds of questions are open and unanswerwed)
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.
You say that in testing, different packages are in different containers, is that true? Where is that defined?
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.
reused? Wouldn't this be "used"?
"script" spelling
"This scrip simply invokes lcov on the overlay workspace after testing is completed ..."