Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

dcou: set_accounts_hash() #32829

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Aug 14, 2023

Problem

context: #32748 (comment)

This PR is a follow up #32822, which showed how to avoid creating a new function just for testing (#32822 (review)).

Summary of Changes

Conditionally make set_accounts_hash() public when the DCOU feature is enabled.

@brooksprumo brooksprumo self-assigned this Aug 14, 2023
@brooksprumo brooksprumo force-pushed the dcou/set-accounts-hash branch from 61bf0a0 to 6c9da95 Compare August 14, 2023 14:46
@brooksprumo brooksprumo added work in progress This isn't quite right yet noCI Suppress CI on this Pull Request labels Aug 14, 2023
@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Merging #32829 (0f9cdb5) into master (de4eee1) will increase coverage by 0.0%.
Report is 1 commits behind head on master.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #32829   +/-   ##
=======================================
  Coverage    82.0%    82.0%           
=======================================
  Files         784      784           
  Lines      211897   211902    +5     
=======================================
+ Hits       173810   173852   +42     
+ Misses      38087    38050   -37     

@brooksprumo brooksprumo force-pushed the dcou/set-accounts-hash branch from 6c9da95 to b056754 Compare August 14, 2023 17:11
@brooksprumo brooksprumo added CI Pull Request is ready to enter CI and removed work in progress This isn't quite right yet noCI Suppress CI on this Pull Request labels Aug 14, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Aug 14, 2023
@brooksprumo brooksprumo force-pushed the dcou/set-accounts-hash branch from b056754 to 0f9cdb5 Compare August 15, 2023 04:09
Copy link
Contributor

@ryoqun ryoqun left a comment

Choose a reason for hiding this comment

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

hehe, i was a bit hesitant to putting u64::default()s at all call sites just for the sake of removing the *_test() version. so i wanted to check that with you before creating a followup pr.

however, seems you're okay with that. :)

@brooksprumo brooksprumo marked this pull request as ready for review August 15, 2023 11:18
@brooksprumo brooksprumo merged commit e316db2 into solana-labs:master Aug 15, 2023
@brooksprumo brooksprumo deleted the dcou/set-accounts-hash branch August 15, 2023 13:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants