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: Update lzv2 README #498

Merged
merged 8 commits into from
Sep 15, 2023
Merged

Conversation

cartyc
Copy link
Contributor

@cartyc cartyc commented Aug 28, 2023

Added details on package structure and new diagram and added current pkg version to kpt commands

@cartyc cartyc requested a review from fmichaelobrien as a code owner August 28, 2023 17:19
johnswayty-ssc
johnswayty-ssc previously approved these changes Aug 28, 2023
Copy link
Collaborator

@davelanglois-ssc davelanglois-ssc left a comment

Choose a reason for hiding this comment

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

@cartyc is there a major reason why you want to embed the package versions in the doc ?

docs/landing-zone-v2/README.md Outdated Show resolved Hide resolved
docs/landing-zone-v2/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@fmichaelobrien fmichaelobrien left a comment

Choose a reason for hiding this comment

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

comma below
two sections, the...

Copy link
Contributor

@fmichaelobrien fmichaelobrien left a comment

Choose a reason for hiding this comment

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

for core-landing-zone and client lz packages - we may want to hyperlink to the packages - to avoid ambiguity

Copy link
Contributor

@fmichaelobrien fmichaelobrien left a comment

Choose a reason for hiding this comment

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

for the deployment set 0.2.0, 0.1.0 and 0.3.2 we may want to document on a line this release set - and that it is a current example - to be modified from in the future

fmichaelobrien
fmichaelobrien previously approved these changes Aug 28, 2023
Copy link
Contributor

@fmichaelobrien fmichaelobrien left a comment

Choose a reason for hiding this comment

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

LGTM - after the hardcoded edits/explanations and minor edits

@cartyc
Copy link
Contributor Author

cartyc commented Aug 29, 2023

@fmichaelobrien added links to the packages in the readme and links to the releases below each package create section.

fmichaelobrien
fmichaelobrien previously approved these changes Aug 31, 2023
Copy link
Contributor

@fmichaelobrien fmichaelobrien left a comment

Choose a reason for hiding this comment

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

LGTM
the readme community will appreciate this

@cartyc
Copy link
Contributor Author

cartyc commented Aug 31, 2023

@davelanglois-ssc added a disclaimer on the package versions. I think keeping the current versions in the commands keeps things easier for now until we can add in a command to grab the correct versions for automation.

fmichaelobrien
fmichaelobrien previously approved these changes Aug 31, 2023
Copy link
Contributor

@fmichaelobrien fmichaelobrien left a comment

Choose a reason for hiding this comment

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

LGTM
Some lint rules are better than others

lucstjean-ssc
lucstjean-ssc previously approved these changes Sep 11, 2023
Copy link
Collaborator

@lucstjean-ssc lucstjean-ssc left a comment

Choose a reason for hiding this comment

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

/lgtm

@cartyc cartyc dismissed stale reviews from lucstjean-ssc and fmichaelobrien via eb52255 September 14, 2023 12:34
Copy link
Contributor

@fmichaelobrien fmichaelobrien left a comment

Choose a reason for hiding this comment

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

LGTM

@cartyc cartyc merged commit d6694fb into GoogleCloudPlatform:main Sep 15, 2023
4 checks passed
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.

5 participants