Skip to content

Conversation

willemneal
Copy link
Contributor

No description provided.

Copy link

netlify bot commented Jul 14, 2025

Deploy Preview for delightful-dieffenbachia-2097e0 canceled.

Name Link
🔨 Latest commit e7a6ca2
🔍 Latest deploy log https://app.netlify.com/projects/delightful-dieffenbachia-2097e0/deploys/688bce8abc242c00088dcf1f

@willemneal willemneal force-pushed the chore/remove_snapshots branch from 8a1ae15 to 05bb8d4 Compare July 14, 2025 23:07
Copy link
Collaborator

@ozgunozerk ozgunozerk left a comment

Choose a reason for hiding this comment

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

I think this will be helpful. We are not utilizing these much, and they are indeed making PRs more noisy

@willemneal
Copy link
Contributor Author

@ozgunozerk Can this be merged?

@ozgunozerk ozgunozerk requested a review from brozorec July 17, 2025 13:44
@ozgunozerk
Copy link
Collaborator

@willemneal LGTM for me, I also asked a review from @brozorec , if he agrees, we can proceed with the merge!

@willemneal willemneal force-pushed the chore/remove_snapshots branch from 05bb8d4 to e7a6ca2 Compare July 31, 2025 20:13
@willemneal
Copy link
Contributor Author

@brozorec It should be good to go! Will make my other PR from being too noisy!

@willemneal
Copy link
Contributor Author

@brozorec Another Ping!

Copy link
Collaborator

@brozorec brozorec left a comment

Choose a reason for hiding this comment

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

@willemneal apologies for the delay here. I wasn't convinced we should remove them altogether as they served me in the past to detect some undesired changes, but I reckon they create more noise than they help. So I'm fine with removing them till we find how to make a good use of them.

@willemneal
Copy link
Contributor Author

I agree it has some use, but when making a big refactor it makes it much harder to review. It might make sense to have a special branch and github action that tests if there is a difference with the latest main branch.

@brozorec Okay to merge?

@ozgunozerk ozgunozerk merged commit baf4b9c into OpenZeppelin:main Aug 8, 2025
6 checks passed
@willemneal willemneal deleted the chore/remove_snapshots branch August 8, 2025 18:02
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