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

WIP: Support nested partitions and partitions directly targeting services #122

Merged
merged 17 commits into from
Feb 7, 2019

Conversation

LPGhatguy
Copy link
Contributor

@LPGhatguy LPGhatguy commented Feb 5, 2019

Closes #112. Closes #102. Closes #105.

This makes the project format significantly easier to reason about by turning both SourceProjectNode and ProjectNode into structs instead of enums with associated data, like they are today.

Error messages should be much better, and it opens up the possibility to specify $className, $children, and $properties on the same partitions that you can specify $path on. It should be a backwards-compatible change, except in the case of previously malformed projects.

TODO

  • ~Custom Deserialize impl to give better error messages on project structure (guide)
  • Documentation on correct project structure Docs need more visiting than just this
  • More exhaustive tests
  • Fix snapshot system blasting away children

Major Changes

  • Removed PathMap's entry API because it wasn't sound
  • Collapsed SourceProjectNode and ProjectNode from enums into structs with optional fields
  • Replaced 'metadata per path' concept with just a set of IDs that a path update affects
  • Removed the concept of snapshot context, which makes snapshotting pure now (!!)
  • Made snapshots correctly encode the project nodes they came from in order to correctly restore children and properties defined in-project
  • Implemented a snapshot testing system that commits expected outputs into source control and compares them at test time. This required creating a partial ordering for some objects to make output more deterministic.

@coveralls
Copy link

coveralls commented Feb 5, 2019

Coverage Status

Coverage remained the same at 80.952% when pulling 0cc79ce on nested-partitions-and-stuff into 38e3c19 on master.

@LPGhatguy
Copy link
Contributor Author

It looks like we might have to start storing actual ProjectNode objects in the path-per-instance metadata object so that we can correctly reconstruct children and properties specified by the project on each change.

@LPGhatguy
Copy link
Contributor Author

Build looks like it might be failing due to how pretty assertions does its comparison?

@LPGhatguy LPGhatguy merged commit ecb9b5e into master Feb 7, 2019
@LPGhatguy LPGhatguy deleted the nested-partitions-and-stuff branch February 7, 2019 22:55
Dekkonot pushed a commit to UpliftGames/rojo that referenced this pull request Jan 11, 2024
…ojo-rbx#122)

* Do the nested partition thing

* Tidy up touched code

* Add nested partition test project, not fully functional

* Clean up variable names, move path_metadata mutation strictly into snapshot_reconciler

* Remove path_metadata, snapshotting is now pure

* Factor out snapshot metadata storage to fix a missing case

* Pull instance_name out of per_path_metadata, closer to what we need

* Refactor to make metadata make more sense, part one

* All appears to be well

* Cull 'metadata_per_path' in favor of 'instances_per_path'

* Remove SnapshotContext

* InstanceMetadata -> PublicInstanceMetadata in web module

* Build in snapshot testing system for testing... snapshots?

* Remove pretty_assertions to see if it fixes a snapshot comparison bug

* Reintroduce pretty assertions, it's not the cause of inequality

* Fix snapshot tests with custom relative path serializer
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.

2 participants