Demo: merge authoring#4083
Draft
ddbeck wants to merge 2 commits into
Draft
Conversation
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
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.
This PR demonstrates one way to author merges, as part of the aspects project work. It assumes compatibility with #4034 and #4079; if you haven't seen those, you should review them first!
Because this PR is partly based on #4079, reviewing 075fac9 shows the actual authoring differences. I'll rebase this PR as soon as the other one merges.
Synopsis
Merges are declared by adding
kind: mergedandredirect_target: <target-feature-id>to a feature description. That's the authoring change.As a convenience to authors, we'll add (generated) comments to the merge targets, to note when other features have been merged into it.
Merged features' BCD keys are implicitly shared into the target feature; you can see the result of this in the
.distfile. The "promotion" of keys into merge targets will be subject to some guidelines and automatic enforcement, the most important of which is that you can't merge a feature with a lower status into a feature with a higher status.Feedback I'm looking for
What about….?
Baseline dates. In this regime, Baseline dates don't change upon merging. A feature's "birth date" holds steady, even if other features merge into it. The use of compat sets means that we have the possibility of changing this behavior in the future (e.g., to generate a rolling Baseline date and a historic inception date), but until we have an actual consumer for such data, I am not planning to implement it.
Merge metadata. We might want additional information about the merge, such as a reason or merge date. I'm not rejecting them, but the authoring story for such data is not new and interesting (compare with discouraged features), so I didn't think it would be very helpful to demonstrate them here.
Merging this feature, specifically. I pulled an example from our open issues (specifically, Merge grid-animation into grid once it becomes Baseline Widely Available #2441). Is this a good idea, specifically? I think this is an easy case, but this is a demo of authoring only—I do not intend to merge this PR. We might ultimately adopt merge guidelines that prevent merging
grid-animation(e.g., we might wish to wait some longer interval before merging the feature or to consult with consumers ahead of merging). But it will be easier to demo possible enforcement and guidelines with some of our more complicated cases after we have a notation for feature merging.Alternatives. I tried out other authoring formats, such as inverting merges (e.g., adding a
subsumedarray to the merge target and using convenience comments on the merged features). This was more cumbersome to write and hid the main consequence of merging a feature: changing a feature'skindfrom the default implicit"feature"value.