Repository navigation
Add migrations to ModuleDef - #5953
Open
coolreader18 wants to merge 3 commits into
Open
coolreader18 wants to merge 3 commits into
coolreader18 wants to merge 3 commits into
Conversation
coolreader18
added this pull request to stack #5954
September 16, 2026 21:33
coolreader18
force-pushed
the
noa/migrations-schema
branch
2 times, most recently
from
September 24, 2026 17:16
202b400 to
eeda389
Compare
coolreader18
removed this pull request from stack #5954
September 24, 2026 17:23
coolreader18
changed the base branch from
noa/upd-rust-1.98
to
noa/fix-environment
September 24, 2026 17:24
coolreader18
added this pull request to stack #5982
September 24, 2026 17:24
coolreader18
force-pushed
the
noa/migrations-schema
branch
from
September 24, 2026 17:42
eeda389 to
04a4569
Compare
coolreader18
marked this pull request as ready for review
September 24, 2026 17:43
coolreader18
force-pushed
the
noa/migrations-schema
branch
from
September 24, 2026 19:13
04a4569 to
612bd1b
Compare
coolreader18
requested review from
bfops,
cloutiertyler,
jdetter and
rekhoff
as code owners
September 24, 2026 21:02
coolreader18
force-pushed
the
noa/migrations-schema
branch
from
September 24, 2026 23:05
612bd1b to
f90ea6a
Compare
| /// `AlgebraicTypeRef`s in other sections refer to this typespace. | ||
| /// See [`crate::db::raw_def::v9::RawModuleDefV9::typespace`] for validation requirements. | ||
| Typespace(Typespace), | ||
| macro_rules! with_v10_sections { |
Contributor
There was a problem hiding this comment.
Why are we introducing a new macro here?
cloutiertyler
left a comment
cloutiertyler
left a comment
Contributor
There was a problem hiding this comment.
What's with all the macro work as part of this PR. It doesn't seem relevant and makes the code considerably more difficult to read.
cloutiertyler
force-pushed
the
noa/migrations-schema
branch
from
September 29, 2026 22:06
f90ea6a to
a7238fe
Compare
coolreader18
removed this pull request from stack #5982
September 30, 2026 18:41
coolreader18
force-pushed
the
noa/migrations-schema
branch
from
September 30, 2026 18:42
a7238fe to
015a1a8
Compare
coolreader18
added this pull request to stack #6026
September 30, 2026 18:46
coolreader18
force-pushed
the
noa/migrations-schema
branch
from
September 30, 2026 18:56
015a1a8 to
044877d
Compare
Contributor
Author
|
The first commit is to simplify the way we modify ModuleDefs - instead of doing a linear search for each one, and having all the different |
coolreader18
force-pushed
the
noa/migrations-schema
branch
from
September 30, 2026 19:14
044877d to
b2d976d
Compare
Contributor
Author
|
It also means we're less likely to accidentally forget about a section, since we can exhaustively destructure the |
coolreader18
force-pushed
the
noa/migrations-schema
branch
from
October 5, 2026 17:33
26dff90 to
b93ecac
Compare
joshua-spacetime
left a comment
joshua-spacetime
left a comment
Collaborator
There was a problem hiding this comment.
Can you add a module def round trip test for a migration:
RawModuleDefV10 → validate → ModuleDef → RawModuleDefV10 → validate
joshua-spacetime
left a comment
joshua-spacetime
left a comment
Collaborator
There was a problem hiding this comment.
Can you generate the module defs for the other languages?
Comment on lines
+7
to
+9
| impl<T> SectionPayload for Vec<T> { | ||
| fn skip_serializing(&self) -> bool { | ||
| self.is_empty() |
Collaborator
There was a problem hiding this comment.
Is this correct to do for empty environments? It's unclear based on this comment:
/// `None` means undeclared; an explicitly empty declaration is `Some(empty)`.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Changes
Adds migrations as a section of ModuleDef. The first commit adds a struct that contains all of the sections, so we don't have to do a linear search each time we modify one.
Rollback safety impact
n/a
Expected complexity level and risk
2
Testing
Stack created with GitHub Stacks CLI • Give Feedback 💬