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

Cleanup serde snapshot's "future" to "newer"#22431

Merged
brooksprumo merged 1 commit intosolana-labs:masterfrom
brooksprumo:rename-serde-snapshot
Jan 13, 2022
Merged

Cleanup serde snapshot's "future" to "newer"#22431
brooksprumo merged 1 commit intosolana-labs:masterfrom
brooksprumo:rename-serde-snapshot

Conversation

@brooksprumo
Copy link
Copy Markdown
Contributor

@brooksprumo brooksprumo commented Jan 11, 2022

As part of #21604, I likely need to add a new Snapshot Version to add accounts_data_len to the snapshot. While starting on that, I found a few bits to cleanup/refactor (based on the original PR #9980).

In particular, I've renamed the "styles" so any mentions of "future" are now "newer" (more details here).

@brooksprumo brooksprumo changed the title Refactor serde snapshot's "future" to "newer" Cleanup serde snapshot's "future" to "newer" Jan 11, 2022
}
match serde_style {
SerdeStyle::Newer => INTO!(TypeContextFuture),
SerdeStyle::Newer => INTO!(newer),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When I add a new case here, the INTO! macro was complaining about using an ident that had scoping (i.e. newer::Context and older::Context). So the simple fix is to have the macro take the serde style, since the Context part is fixed.


mod common;
mod future;
mod newer;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Renamed future to newer, which caused the file to be renamed as well.

@brooksprumo brooksprumo marked this pull request as ready for review January 11, 2022 21:27
@brooksprumo brooksprumo requested a review from mvines January 11, 2022 21:27
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 11, 2022

Codecov Report

Merging #22431 (c5cee6a) into master (637e366) will decrease coverage by 0.0%.
The diff coverage is 87.5%.

@@            Coverage Diff            @@
##           master   #22431     +/-   ##
=========================================
- Coverage    81.1%    81.1%   -0.1%     
=========================================
  Files         553      553             
  Lines      150439   150439             
=========================================
- Hits       122075   122052     -23     
- Misses      28364    28387     +23     

Copy link
Copy Markdown
Contributor

@mvines mvines left a comment

Choose a reason for hiding this comment

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

nice cleanup

@brooksprumo brooksprumo merged commit 9c3144e into solana-labs:master Jan 13, 2022
@brooksprumo brooksprumo deleted the rename-serde-snapshot branch January 13, 2022 13:12
mergify Bot pushed a commit that referenced this pull request Feb 1, 2022
(cherry picked from commit 9c3144e)

# Conflicts:
#	runtime/src/serde_snapshot.rs
mergify Bot added a commit that referenced this pull request Feb 1, 2022
* Refactor serde snapshot's "future" to "newer" (#22431)

(cherry picked from commit 9c3144e)

# Conflicts:
#	runtime/src/serde_snapshot.rs

* fixup conflicts

* fixup remove unused use

Co-authored-by: Brooks Prumo <brooks@solana.com>
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.

2 participants