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.

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

Generate content even for container preview #375

Merged
merged 2 commits into from
Oct 23, 2023
Merged

Generate content even for container preview #375

merged 2 commits into from
Oct 23, 2023

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Feb 19, 2023

This change makes sure that generated content is up to date when previewing locally in a container.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 19, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 19, 2023
@sftim
Copy link
Contributor Author

sftim commented Feb 19, 2023

I'm going to use this as a basis for my work on #374 (whether we merge it or not).

@sftim sftim mentioned this pull request Feb 28, 2023
Merged
sftim
sftim commented Apr 18, 2023
View reviewed changes
hack/gen-content.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed this script so that - at least, in theory - it works equally well for a local build as for a container preview.

@sftim
Copy link
Contributor Author

sftim commented May 24, 2023

💭 do we always want this behavior, or (eg) only when a particular variable is set?

@jberkus
Copy link
Contributor

jberkus commented Jun 7, 2023

What's the status of this PR? It doesn't sound like you're ready with it.

@sftim sftim force-pushed the 20230219_generate_content_container_preview branch from 58c9d93 to a5d0e69 Compare August 21, 2023 16:01
@sftim
Write diagnostics to standard error
71e99b4
@sftim
Generate content also for local container preview
5b8bd15 
This change makes sure that generated content is up to date when
previewing.

Optional future work: allow skipping generation as an opt-in
optimization.
@sftim sftim force-pushed the 20230219_generate_content_container_preview branch from a5d0e69 to 5b8bd15 Compare August 25, 2023 14:47
@sftim
Copy link
Contributor Author

sftim commented Aug 25, 2023 •
edited
Loading

This works for me @jberkus

I don't know whether it's useful for other contributors, and I can't easily answer that question.

I'll rebase it I rebased it against master

@jberkus
Copy link
Contributor

jberkus commented Oct 23, 2023

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 23, 2023
@k8s-ci-robot k8s-ci-robot merged commit 696c914 into kubernetes:master Oct 23, 2023
5 checks passed
@sftim sftim deleted the 20230219_generate_content_container_preview branch October 23, 2023 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jberkus jberkus Awaiting requested review from jberkus

@jeefy jeefy Awaiting requested review from jeefy

Assignees

@jberkus jberkus

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sftim @k8s-ci-robot @jberkus