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

Clarify ECR Auto-login note, fix CanonicalImageName typos #219

Merged
merged 1 commit into from
Jan 31, 2022

Conversation

kingdonb
Copy link
Member

@kingdonb kingdonb commented Jan 14, 2022

Leans on:

Slimmed-down replacement version of #193 that takes into account most other things that were to be fixed here, are fixed by fluxcd/website#702

There was a usability issue reported in the docs that I meant to address here, and it's not obvious from the structure of the change, but as the ImageRepositories controller API docs are not obviously associated with the image-reflector-controller where they are most accessible (on the website, here in the API docs)

There is no breadcrumb which says image-reflector-controller visible anywhere on the page, so it's necessary to spell out which controller needs to get the flag.

In fluxcd/website#702 a detailed guide is added that covers ECR, as well as the other cloud providers, and so this PR can omit the example of how to set up the patch in deference to a link to the guide instead. 👍

@kingdonb
Copy link
Member Author

kingdonb commented Jan 14, 2022

It will be hard to make more progress on this PR before fluxcd/website#702 is merged (and in turn, #194), so it can be reviewed in context.

relu
relu previously requested changes Jan 20, 2022
docs/spec/v1beta1/imagerepositories.md Outdated Show resolved Hide resolved
@stefanprodan stefanprodan added the area/docs Documentation related issues and pull requests label Jan 27, 2022
@kingdonb
Copy link
Member Author

Rebased to get this in shape to merge 👍

@kingdonb kingdonb force-pushed the docs-patches branch 2 times, most recently from c0f70c1 to d61c863 Compare January 27, 2022 16:59
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @kingdonb

@stefanprodan stefanprodan dismissed relu’s stale review January 27, 2022 17:02

We now link to the guide

Link to the Image Update guide

When fluxcd/website#702 merges, this patch will
have been incorporated into the Image Update Guide. Add a link to the
more directly relevant Image Update Guide here instead, since it has the
example, and it doesn't belong in the API docs nor need to be repeated
here, per the discussion from the earlier PR number fluxcd#193.

Signed-off-by: Kingdon Barrett <kingdon@weave.works>

remove orphaned link URLs

Signed-off-by: Kingdon Barrett <kingdon@weave.works>
Signed-off-by: Kingdon Barrett <yebyen@gmail.com>
@stefanprodan stefanprodan merged commit 1c6ea64 into fluxcd:main Jan 31, 2022
@kingdonb kingdonb deleted the docs-patches branch January 31, 2022 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs Documentation related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants