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

[WIP]Have static knative component versions in hack/, update in automatic PR #2677

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

[WIP]Have static knative component versions in hack/, update in automatic PR #2677

wants to merge 26 commits into from
+555 −63

Conversation

gauron99
Copy link
Contributor

@gauron99 gauron99 commented Jan 29, 2025 •
edited
Loading

Whats changing

  • Remove the functionality that components are always fetched as latest and instead have a static version that is periodically checked via Cron job in Workflow and in case a new latest exists == create a PR with that change against main branch programmatically.
  • Creating a new component-versions.sh file that will hold all of these versions that we need in repo (in hack/) and can be this file can be changed via the workflows.
  • hack/main.go created which will delegate all other hack/*.go scripts in the future -- recoding existing hack/*.js ones are on a TODO list as well
  • minor changes regarding the go-github version to v68 to to update deprecated functions

Followup intention

  • New workflow that runs with nightly eventing and serving so we can check against latest earlier and catch possible errors (such as version missmatches etc.) sooner.
  • I plan to add actual tests when more go files are being included / existing ones rewritten in go. Add tests for hack/ directory #2687

todo

  • sync go-github versions if possible to v68
  • hack/main.go to delegate other scripts in hack/
  • Check if PR already exists (update that?)
  • create the actual PR
  • update-knative-components.go complete
  • clean up

fixes #2687

@knative-prow Knative Prow
Copy link

knative-prow bot commented Jan 29, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 29, 2025
@knative-prow knative-prow bot requested review from dsimansk and vyasgun January 29, 2025 19:50
@knative-prow Knative Prow
Copy link

knative-prow bot commented Jan 29, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gauron99

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

@knative-prow knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 29, 2025
@codecov Codecov
Copy link

codecov bot commented Jan 29, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 65.33%. Comparing base (6c57bfa) to head (f2fb6d0).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
pkg/git/github/github.go 0.00% 7 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2677      +/-   ##
==========================================
+ Coverage   64.08%   65.33%   +1.24%     
==========================================
  Files         131      131              
  Lines       15601    15583      -18     
==========================================
+ Hits         9998    10181     +183     
+ Misses       4652     4415     -237     
- Partials      951      987      +36
Flag Coverage Δ
e2e-test 35.84% <0.00%> (+0.07%) ⬆️
e2e-test-oncluster 34.20% <0.00%> (+1.33%) ⬆️
e2e-test-oncluster-runtime 28.55% <0.00%> (?)
e2e-test-runtime-go 26.46% <0.00%> (?)
e2e-test-runtime-node 25.86% <0.00%> (?)
e2e-test-runtime-python 25.86% <0.00%> (?)
e2e-test-runtime-quarkus 26.01% <0.00%> (?)
e2e-test-runtime-rust 24.96% <0.00%> (?)
e2e-test-runtime-springboot 25.00% <0.00%> (?)
e2e-test-runtime-typescript 25.97% <0.00%> (?)
integration-tests 51.94% <0.00%> (+2.20%) ⬆️
unit-tests 50.92% <0.00%> (?)
unit-tests-macos-latest ?
unit-tests-ubuntu-latest ?
unit-tests-windows-latest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@knative-prow knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 30, 2025
gauron99 added 11 commits February 4, 2025 18:39
@gauron99
gomod
1cf3576 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
create hack/main.go, delegate, init dynamic updatic of version in all…
e7a295b 
…ocate.sh, bump github version, unify those versions to v68

Signed-off-by: David Fridrich <[email protected]>
@gauron99
new file, fix, more updates
dfe923a 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
fix
4a6539a 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
bump
2ea8a8b 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
fix
895dc3f 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
simplify
734ea88 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
pr exists, check
b18e41f 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
simplify, fix some stuff
97ec17f 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
simplify further
6b02b2c 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
fix sed error, more comments
6aaa358 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
return to main, dont exit
4199ce7 
Signed-off-by: David Fridrich <[email protected]>
@gauron99 gauron99 requested a review from matejvasek February 4, 2025 18:38
@gauron99 gauron99 marked this pull request as ready for review February 4, 2025 18:39
@knative-prow knative-prow bot requested review from dsimansk and rhuss February 4, 2025 18:39
@gauron99 gauron99 changed the title [WIP] Have static knative component versions in hack/, update in automatic PR Feb 4, 2025
@gauron99
Copy link
Contributor Author

gauron99 commented Feb 5, 2025

PTAL @lkingland @matejvasek

@gauron99
fix printing because no 'ready' is at the end
e3e025b 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
misspell
784d546 
Signed-off-by: David Fridrich <[email protected]>
@gauron99 gauron99 mentioned this pull request Feb 5, 2025
Open
lkingland
lkingland reviewed Feb 5, 2025
View reviewed changes
Copy link
Member

@lkingland lkingland left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looking good! See comments inline where I think perhaps leaning on Go's struct serialization and templating could remove the complexities of file parsing and sed-fu, respectively.

hack/update-knative-components.go Outdated Show resolved Hide resolved
hack/update-knative-components.go Outdated Show resolved Hide resolved
hack/update-knative-components.go Outdated Show resolved Hide resolved
@gauron99
context moved to main
b581f13 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
updt-builder
97186a4 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
update-knative-components fixes, pagination, simplify using go templa…
736db88 
…tes, json file as source of truth

Signed-off-by: David Fridrich <[email protected]>
@gauron99
fix error check
2b5d2b3 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
add json, add kind version, comments
2d8b14e 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
fix versions json
8028be8 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
add newline
3672dcc 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
Copy link
Contributor Author

Ill add a new line at the end of Marshalling the json so reviewdog doesnt return an error

lkingland
lkingland reviewed Feb 11, 2025
View reviewed changes
Copy link
Member

@lkingland lkingland left a comment

Choose a reason for hiding this comment

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

Looking good!
The only bug I see is a missing KindNode member of the struct. Other comments are just suggestions 👍🏻

hack/component-versions.sh Show resolved Hide resolved
hack/update-knative-components.go Outdated Show resolved Hide resolved
hack/update-knative-components.go Outdated Show resolved Hide resolved
hack/update-knative-components.go Outdated Show resolved Hide resolved
hack/update-knative-components.go Outdated Show resolved Hide resolved
hack/update-knative-components.go Outdated Show resolved Hide resolved
@knative-prow knative-prow bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 13, 2025
@gauron99
fix comment inline
1b6470d 
Signed-off-by: David Fridrich <[email protected]>
@gauron99
redo the structure to use cmd/ instead of big hack/main, separate, ad…
816928b 
…d \n to generated .json

Signed-off-by: David Fridrich <[email protected]>
@gauron99 gauron99 changed the title Have static knative component versions in hack/, update in automatic PR [WIPHave static knative component versions in hack/, update in automatic PR Feb 14, 2025
@gauron99 gauron99 changed the title [WIPHave static knative component versions in hack/, update in automatic PR [WIP]Have static knative component versions in hack/, update in automatic PR Feb 14, 2025
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 14, 2025
@gauron99
simplify test, fix linter errors
f2fb6d0 
Signed-off-by: David Fridrich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matejvasek matejvasek matejvasek left review comments

@lkingland lkingland lkingland left review comments

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add tests for hack/ directory
3 participants
@gauron99 @matejvasek @lkingland