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

fix: Updates to proposal 1 #102

Merged
merged 8 commits into from
Jun 28, 2024
Merged

fix: Updates to proposal 1 #102

merged 8 commits into from
Jun 28, 2024

Conversation

rossf7
Copy link
Contributor

@rossf7 rossf7 commented Jun 4, 2024

What type of PR is this?

kind/documentation

What this PR does / why we need it:

Update proposal 1 to

  • Make releases check weekly instead of hourly. As we feel hourly is too often.
  • Use releases endpoint instead of Atom feed
  • Use JSON not YAML for projects metadata to avoid need for Go YAML library

Also adds missing link to atom feed.

Which issue(s) this PR fixes:

None

Special notes for your reviewer (optional):

cc @dipankardas011

@rossf7 rossf7 requested a review from a team as a code owner June 4, 2024 14:52
@dipankardas011
Copy link
Contributor

@rossf7 as you are adding the atom release here may be we can change the purpose of this pr itself if its okay?
and make the necessary changes to accommodate for the jq one
WDYT?

Signed-off-by: Ross Fairbanks <[email protected]>
@rossf7 rossf7 changed the title fix: Make releases check weekly and add atom link fix: Make releases check weekly and use REST API for releases Jun 5, 2024
@rossf7
Copy link
Contributor Author

rossf7 commented Jun 5, 2024

as you are adding the atom release here may be we can change the purpose of this pr itself if its okay?

@dipankardas011 Yes sure. Done

Copy link
Contributor

@dipankardas011 dipankardas011 left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Ross Fairbanks <[email protected]>
@rossf7 rossf7 changed the title fix: Make releases check weekly and use REST API for releases fix: Updates to proposal 1 Jun 14, 2024
Copy link
Contributor

@dipankardas011 dipankardas011 left a comment

Choose a reason for hiding this comment

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

Lgtm

Copy link
Contributor Author

@rossf7 rossf7 left a comment

Choose a reason for hiding this comment

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

Updating the spec after pairing session with @dipankardas011 to simplify the design.

Changes are

  • We no longer use a github var to store the latest release
  • Instead we trigger the pipeline once a day for the latest version of Falco

Not needing to maintain state simplifies and allows us to use bash for the workflow 🎉

@nikimanoledaki @AntonioDiTuri PTAL 🙏

Copy link
Contributor

@dipankardas011 dipankardas011 left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Ross Fairbanks <[email protected]>
Copy link
Contributor

@nikimanoledaki nikimanoledaki left a comment

Choose a reason for hiding this comment

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

LGTM 👍

CI is failing with:

Run bash project-trigger.sh
GH_TOKEN not set
Error: Process completed with exit code 20.

Should the trigger script be running on PRs?

docs/proposals/proposal-001-trigger-and-deploy.md Outdated Show resolved Hide resolved
@rossf7 rossf7 merged commit eea5b5e into cncf-tags:main Jun 28, 2024
1 of 2 checks passed
@rossf7 rossf7 deleted the fix/update-prop-1 branch June 28, 2024 14:05
@rossf7
Copy link
Contributor Author

rossf7 commented Jun 28, 2024

@nikimanoledaki @dipankardas011

Thank you! I'll create an implementation issue for the renaming.

Should the trigger script be running on PRs?

No I think it should only run if there is a change to scripts dir.

Proposed a fix in #109

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.

None yet

3 participants