Skip to content

/ ignition

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

update docs for 3.0.0 #788

Merged
merged 8 commits into from Apr 26, 2019
Merged

update docs for 3.0.0 #788

merged 8 commits into from Apr 26, 2019

Conversation

@ajeddeloh
Copy link
Contributor

ajeddeloh commented Apr 8, 2019

Very rough right now, not worth reviewing yet.
@ajeddeloh ajeddeloh force-pushed the ajeddeloh:more-correct-docs branch from d2bbe66 to d562f44 Apr 10, 2019
@ajeddeloh ajeddeloh marked this pull request as ready for review Apr 10, 2019
@ajeddeloh ajeddeloh changed the title WIP: update docs for 3.0.0 update docs for 3.0.0 Apr 10, 2019
@ajeddeloh

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

ajeddeloh commented Apr 10, 2019

Partially addresses #705.

Still needs updates to the config spec documentation for what fields are used as IDs and which are immune to duplicate merging.
@jlebon
jlebon reviewed Apr 11, 2019
View changes
doc/examples.md Outdated Show resolved Hide resolved
doc/migrating-configs.md Outdated Show resolved Hide resolved
doc/migrating-configs.md Outdated Show resolved Hide resolved
doc/operator-notes.md Outdated Show resolved Hide resolved
doc/operator-notes.md Outdated Show resolved Hide resolved
doc/operator-notes.md Outdated Show resolved Hide resolved
doc/operator-notes.md Outdated Show resolved Hide resolved
doc/operator-notes.md

Since files, directories, and links all describe filesystem entries can can conflict, these lists are deduplicated across each other. This means a file in child config can replace a link in the parent, or a directory in a child config can replace a file in the parent.

### Configs are merged in a depth first traversal

This comment has been minimized.

Sign in to view
Copy link
@jlebon

jlebon Apr 11, 2019

Member

How about just "Configs are merged in the order they appear"? Seems clearer than "depth first traversal".

This comment has been minimized.

Sign in to view
Copy link
@ajeddeloh

ajeddeloh Apr 11, 2019

Author Contributor

That's more ambiguous. Consider if config A merged B and C, but B merges D and E. The final config looks like:

Final =
M(
    M(
        A,
        M(
            M(B, D),
            E
        )
    )
C)

Basically "order they appear" isn't well defined.
@ajeddeloh ajeddeloh force-pushed the ajeddeloh:more-correct-docs branch from d562f44 to cfe4485 Apr 11, 2019
@ajeddeloh

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

ajeddeloh commented Apr 11, 2019 •
edited

Updated except for the DFS part. I agree its clunky, but I'd rather have that be more exact than less clunky. Will mull over a better way to word it.

Edit: kept the DFS title, but made the explanation clearer.
@ajeddeloh ajeddeloh force-pushed the ajeddeloh:more-correct-docs branch from cfe4485 to 507f924 Apr 11, 2019
@jlebon
jlebon approved these changes Apr 12, 2019
View changes
Copy link
Member

jlebon left a comment

Haven't directly checked that all the nits have been addressed, but I trust you and GitHub's Outdated marker. :)

(On that topic, I like GitHub's new interdiff thing, though rebases make it hard to tell what really changed. In contrast, Homu's autosquash feature lets you have fixup! commits that get squashed on merge. Keeping them separate has the effect that they're kept during PR rebases, making review easier.)
@bgilbert
bgilbert reviewed Apr 12, 2019
View changes
doc/examples.md Outdated Show resolved Hide resolved
doc/getting-started.md Outdated Show resolved Hide resolved
doc/development.md Outdated Show resolved Hide resolved
doc/development.md Outdated Show resolved Hide resolved
doc/migrating-configs.md Outdated Show resolved Hide resolved
doc/migrating-configs.md Outdated Show resolved Hide resolved
doc/operator-notes.md Outdated Show resolved Hide resolved
doc/operator-notes.md Outdated Show resolved Hide resolved
@arithx
arithx reviewed Apr 12, 2019
View changes
doc/examples.md Outdated Show resolved Hide resolved
doc/development.md
@@ -1,6 +1,6 @@
# Development

A Go 1.7+ [environment](https://golang.org/doc/install) and the `blkid.h` headers are required.
A Go 1.11+ [environment](https://golang.org/doc/install) and the `blkid.h` headers are required. Using go 1.10 may work, but isn't directly supported since it does not support go modules.

This comment has been minimized.

Sign in to view
Copy link
@arithx

arithx Apr 12, 2019

Contributor

Should we directly call out go versions 1.9.7+ & 1.10.3+ specifically as potentially working as they have minimal module compatability? (https://github.com/golang/go/wiki/Modules#how-are-v2-modules-treated-in-a-build-if-modules-support-is-not-enabled-how-does-minimal-module-compatibility-work-in-197-1103-and-111)

This comment has been minimized.

Sign in to view
Copy link
@ajeddeloh

ajeddeloh Apr 12, 2019

Author Contributor

I think we actually need 1.10 for other reasons, don't remember what explicitly.

This comment has been minimized.

Sign in to view
Copy link
@ajeddeloh

ajeddeloh Apr 12, 2019

Author Contributor

8fff753#diff-354f30a63fb0907d4ad57269548329e3 Is where we bumped it previously. Unfortunately, not much context there.

This comment has been minimized.

Sign in to view
Copy link
@arithx

arithx Apr 12, 2019

Contributor

iirc we bumped it to those versions to have the two most recent go releases supported, not for a compat reason.
doc/development.md
@@ -114,11 +110,11 @@ Create a new [release checklist](https://github.com/coreos/ignition/issues/new?l

## Marking an experimental spec as stable

When an experimental version of the Ignition config spec (e.g.: `2.3.0-experimental`) is to be declared stable (e.g. `2.3.0`), there are a handful of changes that must be made to the code base. These changes should have the following effects:
When an experimental version of the Ignition config spec (e.g.: `3.1.0-experimental`) is to be declared stable (e.g. `3.1.0`), there are a handful of changes that must be made to the code base. These changes should have the following effects:

This comment has been minimized.

Sign in to view
Copy link
@arithx

arithx Apr 12, 2019

Contributor

Should we switch to generics for version numbers (e.x. X.Y.0-experimental) here like we do in other sections?

This comment has been minimized.

Sign in to view
Copy link
@ajeddeloh

ajeddeloh Apr 12, 2019

Author Contributor

I think this is OK. This section is an overview while the following sections are specific instructions.

This comment has been minimized.

Sign in to view
Copy link
@arithx

arithx Apr 12, 2019

Contributor

My line of thinking was that we wouldn't need to update the text again if we ended up making another major version bump in the future. Either way I don't think it's a major issue either way and am ok with whichever you choose.
doc/migrating-configs.md Outdated Show resolved Hide resolved
doc/operator-notes.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@ajeddeloh ajeddeloh force-pushed the ajeddeloh:more-correct-docs branch 3 times, most recently from ba74a2d to 79c1bfd Apr 12, 2019
@arithx
arithx reviewed Apr 23, 2019
View changes
doc/examples.md Outdated Show resolved Hide resolved
@ajeddeloh ajeddeloh force-pushed the ajeddeloh:more-correct-docs branch from 79c1bfd to 70f7f94 Apr 23, 2019
@ajeddeloh

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

ajeddeloh commented Apr 23, 2019

Fixed the link
@ajeddeloh
doc/examples.md: update for spec 3.0.0
6fd6bbc 
Replace old configs with their equivalents in spec 3.0.0. Reword
explainers to be correct.
@ajeddeloh ajeddeloh force-pushed the ajeddeloh:more-correct-docs branch from 70f7f94 to 791cdf9 Apr 24, 2019
@bgilbert
bgilbert reviewed Apr 25, 2019
View changes
Copy link
Member

bgilbert left a comment

LGTM with a couple nits.
doc/development.md Outdated Show resolved Hide resolved
doc/getting-started.md Outdated Show resolved Hide resolved
ajeddeloh added 7 commits Apr 8, 2019
@ajeddeloh
doc/getting-started.md: update for spec 3.0.0
58f2a15 
Drop bits that only apply to spec 2.x.y.
@ajeddeloh
doc/development.md: update doc for spec 3.0.0
7b8fb03 
This doc needs more love, but this should be enough to make it at least
correct.
@ajeddeloh
doc/migrating-configs.md: update for spec 3.0.0
4177ee8
@ajeddeloh
test: turn on docs checking
e6d504b 
Now that spec 3.0.0 is stable and the docs are updated, turn on config
checking in the docs.
@ajeddeloh
doc/operator-notes: add section on config merging
8d83725
@ajeddeloh
contributing.md: add pointer to development guide
86effe6
@ajeddeloh
doc: mv spec 3.1.0 -> 3.1.0-experimental
Loading status checks…
b2f7114 
Rename the doc to correctly match the version. This was an oversight
originally.
@ajeddeloh ajeddeloh force-pushed the ajeddeloh:more-correct-docs branch from 791cdf9 to b2f7114 Apr 26, 2019
@ajeddeloh ajeddeloh merged commit 008bd38 into coreos:master Apr 26, 2019
2 checks passed
2 checks passed
Black Box Tests Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bgilbert bgilbert

@arithx arithx

@jlebon jlebon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@ajeddeloh @bgilbert @arithx @jlebon
You can’t perform that action at this time.