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

Add check for the released files #66

Merged
merged 14 commits into from
Nov 20, 2024

Conversation

see-quick
Copy link
Member

@see-quick see-quick commented Nov 19, 2024

This PR adds a check for released files. So if anyone changes the content of examples/, install/ or helm/charts instead of packaging then CI would throw an error.

Note: Currently we do not have a helm-charts directory because such a feature was implemented recently, so I have added TODO: after we release and then we could also include this check.

Solves #59.

@see-quick see-quick added the enhancement New feature or request label Nov 19, 2024
@see-quick see-quick added this to the 0.2.0 milestone Nov 19, 2024
@see-quick see-quick requested a review from a team November 19, 2024 09:51
@see-quick see-quick self-assigned this Nov 19, 2024
@see-quick see-quick linked an issue Nov 19, 2024 that may be closed by this pull request
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
@im-konge im-konge requested review from scholzj and katheris November 19, 2024 12:56
Copy link
Contributor

@katheris katheris left a comment

Choose a reason for hiding this comment

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

It looks like the indentation in the Makefile.os is wrong, but otherwise looks good to me

Signed-off-by: see-quick <[email protected]>
Signed-off-by: see-quick <[email protected]>
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

Did you actually test the changes? I do not seem to see that in the uild logs.

@see-quick
Copy link
Member Author

see-quick commented Nov 20, 2024

Did you actually test the changes? I do not seem to see that in the uild logs.

./.azure/scripts/release_files_check.sh
checksum of ./install matches expected checksum => OK
checksum of ./examples matches expected checksum => OK

Hmm?

@scholzj
Copy link
Member

scholzj commented Nov 20, 2024

Well, you should test that it actually fails when some changes are in the files. So change something in examples and install, commit and push it => see that it fails. And then once you confirm it you can revert it again.

@see-quick
Copy link
Member Author

Well, you should test that it actually fails when some changes are in the files. So change something in examples and install, commit and push it => see that it fails. And then once you confirm it you can revert it again.

I mean this I tested locally, but if you want it that way sure. I can do it

@see-quick
Copy link
Member Author

see-quick commented Nov 20, 2024

Well, you should test that it actually fails when some changes are in the files. So change something in examples and install, commit and push it => see that it fails. And then once you confirm it you can revert it again.

I mean this I tested locally, but if you want it that way sure. I can do it

./.azure/scripts/release_files_check.sh
checksum of ./install matches expected checksum => OK
ERROR checksum of ./examples does not match expected
expected 5329eddeedb33d52e207946b684862db93ebcd84  -
found 06e7bcc6f583cff0f700c5ee415d3627f9d91564  -
if your changes are not related to a release please check your changes into
./packaging/examples
instead of ./examples

if this is part of a release instead update the checksum i.e.
EXAMPLES_CHECKSUM="5329eddeedb33d52e207946b684862db93ebcd84  -"
->
EXAMPLES_CHECKSUM="06e7bcc6f583cff0f700c5ee415d3627f9d91564  -"
make: *** [Makefile:81: release_files_check] Error 1

works now I will revert it.

@scholzj
Copy link
Member

scholzj commented Nov 20, 2024

Now you can test it for the installation files as well :-)

@scholzj
Copy link
Member

scholzj commented Nov 20, 2024

I guess you can think a bit about the output as well?

checksum of ./install matches expected checksum => OK
ERROR checksum of ./examples does not match expected
expected 5329eddeedb33d52e207946b684862db93ebcd84  -
found 06e7bcc6f583cff0f700c5ee415d3627f9d91564  -
if your changes are not related to a release please check your changes into
./packaging/examples
instead of ./examples

if this is part of a release instead update the checksum i.e.
EXAMPLES_CHECKSUM="5329eddeedb33d52e207946b684862db93ebcd84  -"
->
EXAMPLES_CHECKSUM="06e7bcc6f583cff0f700c5ee415d3627f9d91564  -"

The last part does not make sense ... you do not need to change the EXAMPLES_CHECKSUM variable but the checksum file. The rest seems to have a bit storage formatting, but I guess after you read it few times it makes sense.

Signed-off-by: see-quick <[email protected]>
@see-quick
Copy link
Member Author

I guess you can think a bit about the output as well?

checksum of ./install matches expected checksum => OK
ERROR checksum of ./examples does not match expected
expected 5329eddeedb33d52e207946b684862db93ebcd84  -
found 06e7bcc6f583cff0f700c5ee415d3627f9d91564  -
if your changes are not related to a release please check your changes into
./packaging/examples
instead of ./examples

if this is part of a release instead update the checksum i.e.
EXAMPLES_CHECKSUM="5329eddeedb33d52e207946b684862db93ebcd84  -"
->
EXAMPLES_CHECKSUM="06e7bcc6f583cff0f700c5ee415d3627f9d91564  -"

The last part does not make sense ... you do not need to change the EXAMPLES_CHECKSUM variable but the checksum file. The rest seems to have a bit storage formatting, but I guess after you read it few times it makes sense.

Yeah true last part is no needed. Updated to

./.azure/scripts/release_files_check.sh
ERROR checksum of ./install does not match expected
expected 14107f5b805ba8ccceb44f0845d535b8732c2e6e  -
found 9f873fbb2be9bc0ac33b5f32abf353482c7e572c  -
if your changes are not related to a release please check your changes into
./packaging/install
instead of ./install
checksum of ./examples matches expected checksum => OK
make: *** [Makefile:81: release_files_check] Error 1

Signed-off-by: see-quick <[email protected]>
see-quick and others added 2 commits November 20, 2024 16:16
Co-authored-by: Jakub Scholz <[email protected]>
Signed-off-by: Maros Orsak <[email protected]>
Co-authored-by: Jakub Scholz <[email protected]>
Signed-off-by: Maros Orsak <[email protected]>
@see-quick
Copy link
Member Author

see-quick commented Nov 20, 2024

I can also update the operator repo after we merge this PR.

@scholzj scholzj merged commit 9c8e87b into strimzi:main Nov 20, 2024
11 checks passed
scholzj added a commit that referenced this pull request Feb 2, 2025
Signed-off-by: see-quick <[email protected]>
Signed-off-by: Maros Orsak <[email protected]>
Co-authored-by: Jakub Scholz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check the released files against changes outside of release
5 participants