Skip to content
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

Docs: Clarified Migration README: grammar, out-of-date, organization #2141

Merged

Conversation

stockholmux
Copy link
Member

Signed-off-by: Kyle J. Davis [email protected]

Issue number: N/A

Description of changes:

  • Clarified grammar: 'we' was used as a collective for the project and actions taken by the software.
  • Updated tense: Several sections were written as potential future tense into actual present tense
  • Removed reference to 'customer'
  • Removed several sections that seem to be out of date
  • Changed 'options' to 'designs' in context of choices vs configuration

Testing done: Its Markdown. I used my eyeballs.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@stockholmux stockholmux changed the title Clarified README: grammar, out-of-date, organization Clarified Migration README: grammar, out-of-date, organization May 18, 2022
sources/api/migration/README.md Outdated Show resolved Hide resolved
sources/api/migration/README.md Outdated Show resolved Hide resolved
sources/api/migration/README.md Outdated Show resolved Hide resolved
sources/api/migration/README.md Outdated Show resolved Hide resolved
sources/api/migration/README.md Outdated Show resolved Hide resolved

Migration code should not assume that any given keys exist, because migrations will be run on live data (where all keys will likely exist) and on pending data (where none, some, or all keys may exist).
Plus, different variants of Bottlerocket may not have the same keys.

The migration system could deserialize the function outputs into the incoming model types to confirm that the structure is valid; we should prototype this idea because it would add safety.
The migration system deserializes the function outputs into the incoming model types to confirm that the structure is valid.
Copy link
Contributor

Choose a reason for hiding this comment

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

This was more of a TODO comment; my sense is that it's not implemented.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah. Gotcha. Inline it's not super clear that it's a todo.

What if we change the Open questions heading to Open questions and future directions and move this down to that section? That way all the maybe stuff goes into end of the document.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed in 73d2b39

sources/api/migration/README.md Outdated Show resolved Hide resolved
sources/api/migration/README.md Outdated Show resolved Hide resolved
sources/api/migration/README.md Outdated Show resolved Hide resolved
sources/api/migration/README.md Outdated Show resolved Hide resolved
sources/api/migration/README.md Outdated Show resolved Hide resolved
sources/api/migration/README.md Outdated Show resolved Hide resolved
sources/api/migration/README.md Outdated Show resolved Hide resolved
@stockholmux stockholmux force-pushed the ds-migrations-readme-updates branch from 73d2b39 to 21dee00 Compare May 18, 2022 20:29
sources/api/migration/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

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

LGTM except for the typo.

style nit: I prefer to prefix the first commit line with the component that's affected. In this case I'd use "docs: ...".

sources/api/migration/README.md Outdated Show resolved Hide resolved
Co-authored-by: Ben Cressey <[email protected]>
Co-authored-by: Matthew James Briggs <[email protected]>
Signed-off-by: Kyle J. Davis <[email protected]>
@stockholmux stockholmux force-pushed the ds-migrations-readme-updates branch from 96de0c2 to ff312b3 Compare May 19, 2022 20:51
@stockholmux stockholmux changed the title Clarified Migration README: grammar, out-of-date, organization Docs: Clarified Migration README: grammar, out-of-date, organization May 19, 2022
@stockholmux stockholmux merged commit 51a328c into bottlerocket-os:develop May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants