Skip to content

chore(proto): proto.lock update#15214

Closed
parametalol wants to merge 1 commit intomasterfrom
michael/proto-lock-20250507
Closed

chore(proto): proto.lock update#15214
parametalol wants to merge 1 commit intomasterfrom
michael/proto-lock-20250507

Conversation

@parametalol
Copy link
Contributor

@parametalol parametalol commented May 7, 2025

Description

Changes made by protolock commit.

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

CI

@parametalol parametalol requested a review from a team as a code owner May 7, 2025 13:35
sourcery-ai[bot]
sourcery-ai bot reviewed May 7, 2025
View reviewed changes
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @parametalol - I've reviewed your changes - here's some feedback:

  • To improve clarity, please add a brief note to the PR description explaining the reason or trigger for this proto.lock update.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good
Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

dashrews78
dashrews78 requested changes May 7, 2025
View reviewed changes
Copy link
Contributor

@dashrews78 dashrews78 left a comment

Choose a reason for hiding this comment

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

What is this for and why is it needed? Need some context here.

@rhacs-bot
Copy link
Contributor

rhacs-bot commented May 7, 2025

Images are ready for the commit at 543e5d0.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.8.x-642-g543e5d04df.

@codecov
Copy link

codecov bot commented May 7, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.13%. Comparing base (60277d2) to head (543e5d0).
Report is 4 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #15214      +/-   ##
==========================================
- Coverage   49.13%   49.13%   -0.01%     
==========================================
  Files        2557     2557              
  Lines      187809   187809              
==========================================
- Hits        92280    92274       -6     
- Misses      88254    88261       +7     
+ Partials     7275     7274       -1
Flag Coverage Δ
go-unit-tests 49.13% <ø> (-0.01%) ⬇️

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.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@openshift-ci
Copy link

openshift-ci bot commented May 7, 2025

@parametalol: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/gke-scanner-v4-install-tests 543e5d0 link false /test gke-scanner-v4-install-tests
ci/prow/ocp-4-12-scanner-v4-install-tests 543e5d0 link false /test ocp-4-12-scanner-v4-install-tests
ci/prow/ocp-4-18-scanner-v4-install-tests 543e5d0 link false /test ocp-4-18-scanner-v4-install-tests

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@parametalol parametalol changed the title choro(proto): proto.lock update chore(proto): proto.lock update May 11, 2025
@parametalol
Copy link
Contributor Author

parametalol commented May 11, 2025 •
edited
Loading

@dashrews78,

What is this for and why is it needed? Need some context here.

Whenever we do a proto change, Style check would complain about proto lock not being up-to-date:
https://github.com/stackrox/stackrox/actions/runs/14893151817/job/41829921562?pr=15058

I found that the proto.lock is not up-to-date with the source on master by some reason, and my #15058 causes some unrelated changes in proto.lock after protolock commit. So this PR just commits the current state.

@dashrews78
Copy link
Contributor

dashrews78 commented May 12, 2025

@dashrews78,

What is this for and why is it needed? Need some context here.

Whenever we do a proto change, Style check would complain about proto lock not being up-to-date: https://github.com/stackrox/stackrox/actions/runs/14893151817/job/41829921562?pr=15058

I found that the proto.lock is not up-to-date with the source on master by some reason, and my #15058 causes some unrelated changes in proto.lock after protolock commit. So this PR just commits the current state.

Yeah I'm aware of the style check. I'm the one that made it flag like this. Which is why I find these changes odd. I also updated the proto.lock last week and it didn't flag any of the changes in this PR. If it was out of date on master, CI would flag it for everyone.

It is supposed to fail on your PR because you changed a storage proto and thus MUST update proto.lock there not separately. My concern is that these changes that are unrelated to anything. If you look at the style job on this PR, it is now failing on basically all the changes that it made. So I'm wondering if you generated it with a version of the proto tools that do not match what master and others are using. That could take us down 2 paths.

  1. you need to use a different version of the proto tools to update the proto.lock on the PR where you changed the storage proto
    OR
  2. we need to update the proto tools on master and that we have everyone use.

I would say #1 is certain and you should only update it there. If we need #2 we should probably save that for 4.9A.

@parametalol
Copy link
Contributor Author

parametalol commented May 13, 2025

I would say #1 is certain and you should only update it there. If we need #2 we should probably save that for 4.9A.

Thanks, understood. I'm closing the PR.

@parametalol parametalol deleted the michael/proto-lock-20250507 branch September 16, 2025 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sourcery-ai sourcery-ai[bot] sourcery-ai[bot] left review comments

@dashrews78 dashrews78 dashrews78 requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@parametalol @rhacs-bot @dashrews78