Skip to content

Deploy and Manage: ECK #380

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 78 commits into from
Feb 18, 2025
Merged

Deploy and Manage: ECK #380

merged 78 commits into from
Feb 18, 2025

Conversation

eedugon
Copy link
Contributor

@eedugon eedugon commented Feb 10, 2025 •
edited
Loading

eedugon
eedugon commented Feb 10, 2025
View reviewed changes
@eedugon eedugon changed the title Deploy and Manage: ECK post-migration clean up and organization Deploy and Manage: ECK Feb 10, 2025
eedugon
eedugon commented Feb 12, 2025
View reviewed changes
shainaraskas
shainaraskas reviewed Feb 12, 2025
View reviewed changes
Copy link
Collaborator

@shainaraskas shainaraskas left a comment

Choose a reason for hiding this comment

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

I'm through the install section.

I wouldn't consider any of these comments blocking. I would recommend that you try to address the ones for the section overview sooner rather than later though (i.e. the links to the other deploy and manage sections applicable to k8s)

shainaraskas
shainaraskas reviewed Feb 12, 2025
View reviewed changes
Copy link
Collaborator

@shainaraskas shainaraskas left a comment

Choose a reason for hiding this comment

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

looks great!

I feel like there are some dragons in the configure deployments section and I put some comments down regarding that.

imo none of this is blocking, especially because a lot of my feedback is style related. we should make some decisions about the pages that might belong in the security area though

@eedugon eedugon requested a review from shainaraskas February 17, 2025 07:08
shainaraskas
shainaraskas previously approved these changes Feb 18, 2025
View reviewed changes
Copy link
Collaborator

@shainaraskas shainaraskas left a comment

Choose a reason for hiding this comment

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

the biggest problem I see now is the inconsistent use of variables, but we don't have to fix that here. I'll open a PR later to clean up variable usage in the entire section.

one problem with one tip then I think we're good

deploy-manage/deploy/cloud-on-k8s/configure-deployments.md
Comment on lines 33 to 35
::::{important}
Explore the [Security](/deploy-manage/security.md) section for configuration options such as user and role management, configuring security realms, securing HTTP communications, and more.
::::
Copy link
Collaborator

Choose a reason for hiding this comment

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

security section doesn't have user and role management or security realms. users and roles does

@shainaraskas shainaraskas merged commit 954a5ae into main Feb 18, 2025
4 checks passed
@shainaraskas shainaraskas deleted the deploy_eck branch February 18, 2025 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shainaraskas shainaraskas shainaraskas approved these changes

@kilfoyle kilfoyle Awaiting requested review from kilfoyle

@kosabogi kosabogi Awaiting requested review from kosabogi

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@eedugon @shainaraskas