Skip to content

Prevent proto package recompiling on every build #941

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

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

Conversation

tr11
Copy link

@tr11 tr11 commented May 3, 2025

Description:
Prevent the hedera and hedera-proto packages from recompiling on every built of dependent crates.

Related issue(s): None

Fixes #

Currently the proto package will trigger a recompilation of both hedera and hedera-proto packages even if nothing changed in the protobufs folder. This PR changes the emit_rerun_if_changed behavior before compiling the modified proto files from services folder to ensure reruns only trigger when there are modifications.

Notes for reviewer:

Test by compiling twice any crate dependent on this package.

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@tr11 tr11 requested review from a team as code owners May 3, 2025 16:03
@tr11 tr11 requested review from Sheng-Long and RickyLB May 3, 2025 16:03
@gsstoykov
Copy link
Contributor

@tr11 Thanks for the contribution! DCO checks will fail when commits are not signed. Take a look at https://docs.github.com/en/authentication/managing-commit-signature-verification/generating-a-new-gpg-key

@tr11 tr11 force-pushed the fix-proto-recompiling branch 2 times, most recently from d9d6ce1 to 13639aa Compare May 9, 2025 22:27
@tr11 tr11 force-pushed the fix-proto-recompiling branch from 13639aa to 6f033d5 Compare May 9, 2025 22:28
@tr11 tr11 force-pushed the fix-proto-recompiling branch from fedcef3 to 1884cee Compare June 27, 2025 01:31
@mehcode
Copy link
Contributor

mehcode commented Aug 9, 2025

Can we please get this merged?

And if there is anyway to add a test to ensure this behavior isn't broken, can we please do that as well. Our team fixed this last year ( #864 ) but it broke a few months later and we haven't had the chance to fix it again. Thanks @tr11.

@gsstoykov
Copy link
Contributor

@mehcode @tr11 PR needs to be formatted. Will try to get this in ASAP.

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.

3 participants