Conversation
…accumulation test
chrismacklin
left a comment
There was a problem hiding this comment.
Let's talk over these comments before continuing.
| type PragmaValidationResult = Result<unit,string> | ||
|
|
||
| type PragmaPersistence = | Persistent | Transient | ||
| type PragmaPersistence = | Persistent | Transient | TransientCumulative |
There was a problem hiding this comment.
I'm concerned that this is now mixing two orthogonal concerns; whether or not a pragma is transient or not is orthogonal to whether or not a pragma should accumulate arguments when multiple instances of it appear.
Probably best thing to do here is refactor this to remove the additional variant from PragmaPersistence and add a field on PragmaDef, perhaps accumulateArgs: bool. That should simplify the implementation of Add below.
| @@ -0,0 +1,90 @@ | |||
| /// Assembly transforming plugin that implements seamless part assembly. | |||
There was a problem hiding this comment.
I not sure if a plugin is appropriate for implementing this feature. We had to extend the static type signature of assembly to accommodate this feature, so it seems to me like we should integrate the tagging addition directly into the assembly transformation stage. There is already a list of default assembly transformations that always happen and I think this is appropriate to add there.
Hopefully this one is a little simpler than last PR. This adds the
#tag namespace:tagfeature and extends pragma semantics to add a transient cumulative option.