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

Refactor - Demotes Arc to Rc. #32982

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

Lichtso
Copy link
Contributor

@Lichtso Lichtso commented Aug 24, 2023

Problem

Another preparation for #32961.

Future versions of clippy will complain that:

warning: usage of an Arc that is not Send or Sync
= note: the trait Sync is not implemented
= note: required to implement Send and Sync
= help: consider using an Rc instead or wrapping the inner type with a Mutex
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync

Summary of Changes

Demotes Arc to Rc around RemoteWalletManager, LedgerWallet and TransactionAccounts.

brooksprumo
brooksprumo previously approved these changes Aug 24, 2023
@Lichtso
Copy link
Contributor Author

Lichtso commented Aug 24, 2023

Sorry @brooksprumo, forgot to run cargo fmt

apfitzge
apfitzge previously approved these changes Aug 24, 2023
Copy link
Contributor

@apfitzge apfitzge left a comment

Choose a reason for hiding this comment

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

Lgtm

@@ -98,8 +99,8 @@ pub struct RemoteWalletManager {
impl RemoteWalletManager {
/// Create a new instance.
#[cfg(feature = "hidapi")]
pub fn new(usb: Arc<Mutex<hidapi::HidApi>>) -> Arc<Self> {
Arc::new(Self {
pub fn new(usb: Arc<Mutex<hidapi::HidApi>>) -> Rc<Self> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Potentially we should have been returning some sort of type aliased thing here?

That way we wouldn't have had so many changes from Arc -> Rc

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #32982 (b437b74) into master (68456fa) will increase coverage by 0.0%.
The diff coverage is 66.6%.

@@           Coverage Diff           @@
##           master   #32982   +/-   ##
=======================================
  Coverage    81.9%    82.0%           
=======================================
  Files         784      784           
  Lines      212715   212715           
=======================================
+ Hits       174426   174431    +5     
+ Misses      38289    38284    -5     

@Lichtso Lichtso merged commit 67d6d68 into solana-labs:master Aug 24, 2023
13 checks passed
@Lichtso Lichtso deleted the refactor/demote_arc_to_rc branch August 24, 2023 22:54
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