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

Feature installation order #43

Closed
chrmarti opened this issue Jun 16, 2022 · 26 comments · Fixed by #48
Closed

Feature installation order #43

chrmarti opened this issue Jun 16, 2022 · 26 comments · Fixed by #48
Assignees
Labels
proposal Still under discussion, collecting feedback

Comments

@chrmarti
Copy link
Contributor

chrmarti commented Jun 16, 2022

As we are preparing to open up features to community contributions, we found that some features need to install after other features. This issue proposes to add support for soft-dependencies to arrive at a reproducible ordering following these ordering requirements:

A feature can declare a list of other features that should be installed first if they are configured to be installed. (The soft-dependency itself does not add another feature to the list of features to be installed.)

E.g., the NodeJS feature would list the Python feature because it has to install a system version of Python if no version of Python is already installed:

{
    "id": "node",
    "installAfter": [ "devcontainers/features/python" ],
    // ...
}

Features that an install in any order will be installed in a fixed order (e.g., ordered by id) to ensure the result is always the same.

The "features" map in the devcontainer.json will remain a map. See https://github.com/devcontainers/spec/pull/27/files#r869018039 for a discussion.

To allow for progress on user contributable features, we can use a stop-gap measure like an ordered list of features that depend on a specific installation order and either bake that into the CLI at build time or fetch it at runtime. This list will be temporary and documented as such.

/cc @Chuxel @jkeech @edgonmsft @joshspicer @bamurtaugh @alexdima

@chrmarti chrmarti added the proposal Still under discussion, collecting feedback label Jun 16, 2022
@chrmarti chrmarti self-assigned this Jun 16, 2022
@Chuxel
Copy link
Member

Chuxel commented Jun 16, 2022

This list will be temporary and documented as such.

@chrmarti I think we'll still need some way to allow users to specify explicit ordering in the feature object in devcontainer.json. Not every feature will know about all other features even long term - and there's other reasons below. Right now this is impossible, which has been problematic for early adopters.

One way to do that could be to allow "installAfter" to also be explicitly mentioned by users in the object syntax in devcontainer.json as well. e.g.

"features": {
    "common": "latest",
    "docker-in-docker": {
        "installAfter": [ "common" ]
    }
}

This would enable similar ordering to an array with a slight variation on syntax for those that value predictability.

Otherwise, I think we'd likely need an array with a mode that allows you to opt into implicit vs explicit ordering, but I don't think we'd want the array syntax to be temporary if that's the direction we go.

To recap, explicit ordering allows end users to:

  1. Have a well-known and predictable outcome as they add more features (given features need to adapt to the image contents). Reproducible image builds is a key best practice for creating container images. An unordered dependency graph is not transparent or predictable over time since execution order influences what each feature does. Adding a new feature to the end of the list will not affect anything above it, while a random order can result in more material changes.
    • Examples include the relationship between the Node and Python features, and the Git from Source feature and most other features. Order can result in different behaviors in installing OS dependencies.
    • Similar problems can occur between features authored by multiple users that have no knowledge of one another – and thus no possible dependency graph.
  2. Have a way for a developer to easily add a local script that runs as a part of this sequence, without getting into dependency management or complex conditional logic that adapts to different ordering or image scenarios.
    • Existing examples include installing the preview dev CLI into the Azure CLI, installing more go tools (via go install), adding desktop customization for the desktop-lite script, etc.
    • A local feature with a single script can be dropped into the ordered execution for this to happen.
    • You could in concept have a local feature that tries to ensure ordering with an "installAfter", but what may actually want is "installBefore" in this case since it's repo specific... otherwise any additions could case the feature to happen at a different spot.
  3. Simplify image build debugging (and fixing) if something goes wrong – whether related to their own features, 3rd party ones, their Dockerfile, or base image. Reordering can reduce duplicate installation and work around bugs, but devs can also add local features as a workaround to install something missing, edit folder structures / add symlinks, etc at the exact right spot in the execution flow.
    • This is partially important when using features from multiple authors.
    • Reordering would otherwise be impossible. You’d have to abandon the features.
    • Like (2), you could in concept have a local feature that tries to fix it with an "installAfter", but what you'd more likely want is "installBefore"... otherwise any additions could case the fix feature to happen at a different spot.

@joshspicer
Copy link
Member

joshspicer commented Jun 16, 2022

I believe we should provide some supported (permanent) mechanism for allowing an end-user to control feature installation. I think it is preferable that a user does not need to concern themselves with install order, but as a "break glass" solution it should be intuitive how to order when the need arises.

Our "first party" features are written in such a way that most installation orders will work (no errors), although in some situations there is a preferred order. Most of the time the preferred order will eliminate the need for a "soft dependency" for a given feature to be installed twice. We try to optimize our scripts to re-use existing tools if present already in the container, but that isn't going to always be trivial to account for. Today we force our preferred order, regardless of any order specified from the user. We can do this since we have full control over the ecosystem at the moment (aside from a few adventurous preview users). That ordering is declared from this file.

When we factor in community ("third party") feature contributions, we now have significantly less control over how the features are implemented. Additionally, we currently don't have a proposal for a centralized "repository" of features, instead opting for a more "Self-hosted" model. We don't plan to strictly moderate the implementation of features, and thus authors are allowed flexibility in authorship. We are pushing a lot of added responsibility onto community authors to "get it right". Many times features will install fine without a specified order, but in the instances where there is an obvious ordering problem that our tooling did not properly infer, I believe the most useful solution we can provide to users for that problem is allowing a specified order.

Not every feature will know about all other features - and there's other reasons below. Right now this is impossible, which has been problematic for early adopters.

I agree. One solution to this is the array notation (proposed here). I believe our tooling can (and should) implement some "ordering logic" that helps prevent a user from hitting obvious/identifiable error, but as a final solution a user should be given the tools to solve a problem themselves that could result from ordering. I especially like @Chuxel 's point (2), where users may want to prototype a feature locally or perform a simple extension of already installed features.

The implementation of container features today is inherently ordered due to the programatic injection of Dockerfile RUN lines for each feature. Preventing a user from altering (or at the very least overriding), will create situations where a user may need to fallback to manually editing their Dockerfile. In my view, the fundamental goal of features is to abstract away (for an end-user) the need to have any dockerfile at all in their project, i n lieu of a single .devcontainer.json.

@kieferrm
Copy link

The argumentation seems to have two sides

  • features are purposefully loosely specified and therefore messy situations will arise
  • by default, users should not have to worry about in what sequence features need to be listed

If this observation is correct, it seems to me that the syntax should explicitly express both of those aspects. I.e. the syntax should not suggest to me that by default I do have to worry about the sequence and when I look at a devcontainer file it should immediately be clear to me that it defines order to clean up a messy situation.

Thus, it would make sense to me that

  1. features define their dependencies
  2. the features of a dev container are defined as a map and are being installed based on 1.
  3. the dev container file may contain a property overrideFeatureInstallOrder (or a different name along those lines) that allows a user to specify the installation order, and the value of this property takes precedence over 1.

@chrmarti
Copy link
Contributor Author

chrmarti commented Jun 17, 2022

A separate overrideFeatureInstallOrder may only work well if we make sure it is only rarely needed and we consider it a bug on the feature that requires the override (or a feature request on us if the feature cannot address it).

On reproducible image builds: We will compute a fixed order based on the soft-dependencies (using a stable sorting where soft-dependencies do not require an order). We will also need a lockfile to make the build reproducible over time as feature scripts update.

On local feature scripts: We can install features that do not participate in the soft-dependencies graph last. They will not participate either on purpose or by accident, running them last should work in both cases. A local feature can then still choose to participate in the soft-dependencies.

@chrmarti
Copy link
Contributor Author

My main worry with overrideFeatureInstallOrder is that it would break the feedback loop we would otherwise get from users and feature authors that would allow us to improve the implementation and API. We could show a warning on overrideFeatureInstallOrder in the editors to counter that.

@Chuxel
Copy link
Member

Chuxel commented Jun 17, 2022

The argumentation seems to have two sides

  • features are purposefully loosely specified and therefore messy situations will arise
  • by default, users should not have to worry about in what sequence features need to be listed

If this observation is correct, it seems to me that the syntax should explicitly express both of those aspects. I.e. the syntax should not suggest to me that by default I do have to worry about the sequence and when I look at a devcontainer file it should immediately be clear to me that it defines order to clean up a messy situation.

Thus, it would make sense to me that

  1. features define their dependencies
  2. the features of a dev container are defined as a map and are being installed based on 1.
  3. the dev container file may contain a property overrideFeatureInstallOrder (or a different name along those lines) that allows a user to specify the installation order, and the value of this property takes precedence over 1.

@kieferrm 100%. I was thinking of a property inline in the object syntax - but something like an override property would work as well. Strict ordering should not be the default, but possible to do. I'd talked with @craiglpeters yesterday on the Codespaces team and we both were leaning towards the object semantic as the default with an override form for the reasons you mention.

My main worry with overrideFeatureInstallOrder is that it would break the feedback loop we would otherwise get from users and feature authors that would allow us to improve the implementation and API. We could show a warning on overrideFeatureInstallOrder in the editors to counter that.

@chrmarti I'm pretty confident that if people find out that they need to use this property frequently that we'll get feedback on something being wrong. The idea really is that you shouldn't have to do this.

The core challenge is that, unlike a classic package or module system, we only have complete control of the "packaging" aspect of the overall process. How the installation itself affects other things is dictated by things like apt, npm, pip, nix, etc. So that inherently puts us in a spot where predictability is difficult.

However, I strongly believe we should look for ways to avoid these problems as well, so agree with that point. I also agree with your thought that we should probably consider a way to make user creation something that can be done independent of features since its placement has a dramatic affect on ordering right now.

On reproducible image builds: We will compute a fixed order based on the soft-dependencies (using a stable sorting where soft-dependencies do not require an order). We will also need a lockfile to make the build reproducible over time as feature scripts update.

Yeah the specific issue is when you add one more thing, that can end up in a (to the user) random spot in the ordering and affect things in ways we cannot predict. In primary cases, that won't be a problem, but we'll need a way to handle exceptions. We've seen enough already at this early stage that having some mechanism will help a lot.

On local feature scripts: We can install features that do not participate in the soft-dependencies graph last. They will not participate either on purpose or by accident, running them last should work in both cases. A local feature can then still choose to participate in the soft-dependencies.

This is a good idea - I like it! The one thing it does not solve is where you need that to happen between two features as a fix, but with an override, this should reduce the number of incidents significantly. Love that.

@joshspicer
Copy link
Member

The core challenge is that, unlike a classic package or module system, we only have complete control of the "packaging" aspect of the overall process.

I want to emphasize this point made by Chuck. We currently are NOT building a package manager (we have no centralized database of features, there is no lock file, we have no proposal for a strict versioning schema other than "what makes sense" to an author). We have also explored building on top of an existing package manager, but @edgonmsft 's investigations concluded it did not make sense given the specific case studies we investigated.

We indeed have control over packaging via our provided GitHub action (https://github.com/microsoft/publish-dev-container-features-action, soon to be migrated over to the devcontainers org following a revised packaging schema). But at packaging time, we again only have a view into the immediate set of features - there is no central "features" database we can query to add metadata).

@joshspicer
Copy link
Member

joshspicer commented Jun 17, 2022

The argumentation seems to have two sides

  1. features are purposefully loosely specified and therefore messy situations will arise
  2. by default, users should not have to worry about in what sequence features need to be listed

I just want to voice that I don't think (2) is necessarily a hard requirement, although that is certainly the way the discussion is heading. I think that our tooling should provide some sortof mechanism for detecting invalid orderings, but installing tools into a Docker image in inherently ordered and thus by us taking the ability to define an order away from a user, we're necessitating moving that complexity onto someone else (feature authors and ourselves as developers of the ordering algorithm).

On local feature scripts: We can install features that do not participate in the soft-dependencies graph last. They will not participate either on purpose or by accident, running them last should work in both cases. A local feature can then still choose to participate in the soft-dependencies.

Agreed, this makes the most sense in a completely opaque ordering model!

Although - if a user wanted to then utilize several "local" features, we run into the exact same problem as if a user wanted to utilize several "remote" features.

Perhaps the core take-away is that we don't have enough user feedback yet on how one wants to utilize and stretch the power we provide with features.

@Chuxel
Copy link
Member

Chuxel commented Jun 17, 2022

I just want to voice that I don't think (2) is necessarily a hard requirement, although that is certainly the way the discussion is heading.

@joshspicer I think this is representative of our aspiration. We have a checkbox UX right now with this in mind... pick your base, then add a few optional features and you are done. Practically speaking we've found it is more complicated than just that, but this is also not an absolute either. Anything that's a straight CLI you curl and drops in /usr/local/bin works pretty well this way. We may want to favor that acquisition path - particularly when signing is available to verify contents like there is for the Go install - since it will be less succeptible to the problem.

The core issue that tends to cause things like Go to need ordering is that it needs to set privs to enable a non-root user to access or install contents in folders it creates. Given how common that is, I could totally see us going down the path where remoteUser supports creating said user rather than just referencing like it is now. That would then happen first, and at that point the remaining challenges would be more exception based as @chrmarti has pointed out in the past.

In fact, we could pass that user name into the features, which would simplify logic we have now to figure out what that user is. (e.g. it's there as a DEV_CONTAINERS_REMOTE_USER env var). This is not really practical with the script form where many of these scripts started, but with dedicated features, it would work really well and also simplify authoring.

@chrmarti
Copy link
Contributor Author

Although - if a user wanted to then utilize several "local" features, we run into the exact same problem as if a user wanted to utilize several "remote" features.

I was thinking that a local feature could also use the "installAfter" property to get in control of the order. We just order features without any dependencies or dependents last to increase the chances of success for features that are unaware of their dependencies (and need to be fixed).

@joshspicer
Copy link
Member

joshspicer commented Jun 17, 2022

I want to publicly share an example proposal of @Chuxel 's shared in a different medium.

There is then a “featureOrder” (or similar) property in devcontainer.json that dictates ordering behaviors. This has three possible values “auto” (the default), “strict”, and “manual”. This is used in conjunction with the array syntax.

In “auto” mode, the dev container CLI will tweak the order in the array based on what is in “runsAfter” for a given feature. Specifically:

For each feature in the array, see if any features listed in “runsAfter” are also in devcontainer.json.

If they are, change the actual ordering to be sure that the current feature runs after each item in this list.

In the event ordering is changed, output a warning that the order was changed in the logs with a note on how to stop this from happening. (e.g., “Change featureOrder to strict to disable this behavior”)

In “manual” mode, the execution order is forced to be exactly what is in the array in devcontainer.json – regardless of whether or not this is the recommended order. If something is out of order based on its “runsAfter” values, a warning is placed in the logs to aid debugging should the build fail unexpectedly due to a feature issue.

In “strict” mode, the execution order is also always based on what is in the array in devcontainer.json. However, if something is out of order based on its “runsAfter” values, the build fails with an explicit error saying what is out of order.

This idea of "modes" auto (default), strict, and manual seems to provide the most power and flexibility to the end-user. Users that want the "magic" experience don't tinker with the featureOrder, and those that identify unsolvable problems (or would prefer to iterate on a feature without worrying about the plethora of dependency models we have discussed), can then change their installation mode.

This seems like a nice compromise. The "magic" dependency detection exploration can continue, while users today are not blocked from using features in a way that works for them.

I expect that users in "auto" mode will have their devcontainers break as the team continues to iterate on automatic feature re-ordering. These "modes" will provide a way for users to unblock themselves and continue to stay in our ecosystem. The alternative would be to leave features altogether in favor of manually writing their Dockerfile.

@chrmarti
Copy link
Contributor Author

I have put together a PR demonstrating an implementation of the proposal: devcontainers/cli#56

@joshspicer
Copy link
Member

Although - if a user wanted to then utilize several "local" features, we run into the exact same problem as if a user wanted to utilize several "remote" features.

I was thinking that a local feature could also use the "installAfter" property to get in control of the order. We just order features without any dependencies or dependents last to increase the chances of success for features that are unaware of their dependencies (and need to be fixed).

Makes sense! In isolation all the local features could then be completely controlled.

@edgonmsft
Copy link
Contributor

Another thing is that going with a map syntax doesn't allow features to be called multiple times, and for the cases where that makes sense then features and their parameters need to be more complicated.

@Chuxel
Copy link
Member

Chuxel commented Jun 17, 2022

Another thing is that going with a map syntax doesn't allow features to be called multiple times, and for the cases where that makes sense then features and their parameters need to be more complicated.

@edgonmsft This does not bother me - features have to be specifically designed to be invoked twice anyway, so supporting a list of things to install really isn't that difficult. It does imply though that we should support an array option type. There's a spike in end user complexity to do this multiple times.

So recapping where we here:

  1. ✅ We agree installAfter should be available in features and this affects ordering.
  2. ✅ We agree local features should run last, unless otherwise dictated by a installAfter property.
  3. ✅ We agree that by default, ordering should be automatic.
  4. ✅ We agree that we should look into a semantic to create users automatically to avoid this common issue - (⚫️) but this is a separate proposal.
  5. ✅ We agree it should be possible for the end user to force a specific order to resolve challenges, however, this should be clear that this is an exception.
  6. ✅ We agree that we need some way of a feature to enable users to specify more than one thing to be installed easily (e.g. an array option). But have not yet settled on a form - (⚫️) though this is a separate proposal.
  7. ✅ We agree that we should favor feature author complexity over end user complexity.
  8. ✅ We agree long term we may need some sort of composite feature semantic - (⚫️) though that's a separate proposal.
  9. ✅ We agree the object syntax for features more naturally conveys automatic ordering.
  10. ✅ We need to confirm our direction on the specific semantics. UPDATE: Semantics covered here with one tweak agreed to here.

@joshspicer
Copy link
Member

joshspicer commented Jun 17, 2022

We have two pending changes in this proposal:

1. The top-level installsAfter array property is introduced. This property is added by feature authors in devcontainer-feature.json definitions.

2. The implicit behavior that "local features" are automatically re-ordered and applied after all "remotely acquired" features.


In addition to (1) and (2), I propose a third (3) optional, "break-glass" top-level setting: featureInstallOrder, that provides feature users the ability to override the "magic"/"inferred" installation order.

The property is an array of unversioned featureIDs. Those IDs must also be present in the features object.

These features will be applied to the underlying Dockerfile for installation in the specified order. Any features in the features object that are not mentioned in the array will be installed in an undefined/implicit order, as determined as optimal by the tooling.

NOTE: In this example, person/features/cool-azure-cli-extension does not specify an installsAfter property and thus would be ordered randomly before any "local features". There are numerous reasons why this could be the case (i.e: (a) inactive maintainer, (b) disagreement on adding in the property by the maintainer, (c) proprietary/closed-source features, (d) this feature only needed to be re-ordered on particular base images, (e) this feature only needs to be re-ordered due to interactions with a different user-defined feature, etc). In the example below, the installation only behaves properly with the prescribed order.

NOTE: "local features" are features referenced from the local filesystem. They are part of the spec and are currently (as of Jun 17 2022) in PR to be added to the reference implementation.

Example

{
    "image": "azcr.io/joshspicer/my-debian",
    "features": {
        "person/features/cool-azure-cli-extension": "",
        "docker-in-docker": {
            "version": "20.01"
        },
        "devcontainers/features/azure-cli": {
            "version": "latest"
        },
        "/home/alice/localFeatures/setupDev": {
            "doComplicatedStep": true
        },
        "/home/alice/localFeatures/setupProxy": {
            "ipAddress": "127.0.0.1"
        },
        "homebrew": {
            "version": "latest"
        },
        "acmecorp/features/proprietaryCode": {
            "optionA": true,
            "optionB": false
        },
        "common": {
           "userUID": "1000"
        }
    },
    "featureInstallOrder": [
        "/home/alice/localFeatures/setupProxy",
        "common",
        "devcontainers/features/azure-cli",
        "person/features/cool-azure-cli-extension"
    ]
}

Thus the resulting Dockerfile will have RUN lines injected with features in the following order:

/home/alice/localFeatures/setupProxy
common
devcontainers/features/azure-cli
person/features/cool-azure-cli-extension            
--------------------------------------------------
acmecorp/features/proprietaryCode
docker-in-docker
homebrew
--------------------------------------------------
/home/alice/localFeatures/setupDev

The three buckets above are:

  1. Explicitly ordered by 'featureInstallOrder'
  2. Default: Remote features randomly ordered by tooling in a smart way (may change over time)
  3. Default: Local features install last by default , randomly ordered by tooling in a smart way (may change over time)

If no featureInstallOrder is provided in the devcontainer.json, just bucketing modes (2) and (3) are applied.

@craiglpeters
Copy link
Member

I like this proposal for featureInstallOrder @joshspicer. Do we have a use case where it is required in order to get the required outcome? Seems to me you could get the resulting Dockerfile RUN using installsAfter.

@joshspicer
Copy link
Member

joshspicer commented Jun 17, 2022

@craiglpeters - We have many use-cases today that have arisen from using features in our own base images. I've also provided 5 generalized example use cases in my first NOTE: above

EDIT: I also want to point out my previous comment, where I highlight that today, all of our first party features follow a strict installation order that cannot be modified by the user. This is to protect the user from doing something invalid. My stance is that it will be exceeding difficult to always auto-determine the correct order, and thus as a "bailout"/"break glass" solution we must provide users the tools to do so. When users dabble in our preview functionality of self-authoring features, "mixing" self-authored and our first party set of features may result in inconsistent behavior as the automatic ordering is iterated on.

@craiglpeters
Copy link
Member

@craiglpeters - We have many use-cases today that have arisen from using features in our own base images. I've also provided 5 generalized examples use cases in my first NOTE: above

Noted! Scroll-back is my friend

@Chuxel
Copy link
Member

Chuxel commented Jun 17, 2022

In addition to this, we also identified four things to split out as separate proposals/discussions:

High confidence proposals:

  1. We agree that we should look into a semantic to create users automatically to avoid this common ordering issue.
  2. We agree that we need some way for feature to enable users a to specify that the feature should do a step or set of steps more than once. (e.g., This could be an array variant for the options map, or an array option type.) But have not yet settled on a form - @joshspicer has a proposal in mind here.

Warrants further discussion:

  1. It might be worth enabling a "lifecycle hook" concept for each feature e.g., _postFeatureCommand and _preFeatureCommand as options that are automatically available to end users. (You can already add these explicitly as options to your own feature... this is more of a convenience.) In concept this will reduce the need for local features, but we are not confident enough that is a "must have" to wrap into this proposal.
  2. Long term we may need some sort of composite feature semantic - though we want to wait to see if this is needed. We do not have enough features implemented to have a clear picture of this particular need.

@joshspicer
Copy link
Member

joshspicer commented Jun 17, 2022

For completeness -- an alternative approach to my proposal above is around providing metadata "hints" in the feature options.

These "hints" would start with a leading _, indicating that the property is reserved for the tooling as metadata, and is not to be injected as an environment variable in the script.

This proposal is inspired by configuring firewall rules on something like an Azure NSG.

NOTE: My proposal above is most likely a better fit for the specification, but I leave the idea here open for possible discussion.

Eg:

{
    "image": "azcr.io/joshspicer/my-crazy-complicated-docker-image",
    "features": {
        "person/features/cool-azure-cli-extension": {
            "_installPriority": 105
        },
        "docker-in-docker": {
            "version": "20.01"
        },
        "devcontainers/features/azure-cli": {
            "_installPriority": 100
        },
    },
    "overrideFeatureInstallationMode": "manual"  // (manual|auto) Default: auto
}

When in manual mode, all features with _installPriority will be installed first in ascending order. If more than one feature indicates the same priority, the tooling will throw a validation error.

For all remaining features, or if we are in the default auto mode, the tooling will auto-select a build order based on internal heuristics.

For this example, the build order would be:

devcontainers/features/azure-cli
person/features/cool-azure-cli-extension
docker-in-docker

@chrmarti
Copy link
Contributor Author

chrmarti commented Jun 20, 2022

I'm missing a way to ensure we or the feature author will learn about users having to use featureInstallOrder. When users have no indication that the use of featureInstallOrder should be the exception, they might never report it and continue to use it even after it is no longer needed (e.g., because the feature updated the installAfter list). More importantly, if we don't get that feedback, feature authors might not learn about issues in their installAfter list.

Possible ways of improving this feedback loop:

  • Drop featureInstallOrder for now and tackle the problems reported by users and feature authors by improving the dependency resolution.
  • Use a clearer name for the property: experimentalFeatureInstallOrder, overrideFeatureInstallOrder, fixFeatureInstallOrder.
  • Show a warning in the editor on the property (though that should probably include a clearer name like above).

@Chuxel
Copy link
Member

Chuxel commented Jun 20, 2022

@chrmarti I don't think we should drop featureInstallOrder, but I think overrideFeatureInstallOrder is a good idea since it is indeed an override. Not sure we need the warning with that TBH. Generally if we do not get feedback, I think that means we've solved the problem TBH.

But, we can also do outreach to verify once we've got more people using it to understand why they are using the property - if they do. For feature authors, I'd be surprised if people did not complain given the use of the property means that default behaviors wouldn't work - and enough people will not read docs that they'll raise issues on it.

@joshspicer
Copy link
Member

overrideFeatureInstallOrder sounds good to me as the name of the property.

@chrmarti
Copy link
Contributor Author

@chrmarti I don't think we should drop featureInstallOrder, but I think overrideFeatureInstallOrder is a good idea since it is indeed an override. Not sure we need the warning with that TBH. Generally if we do not get feedback, I think that means we've solved the problem TBH.

But, we can also do outreach to verify once we've got more people using it to understand why they are using the property - if they do. For feature authors, I'd be surprised if people did not complain given the use of the property means that default behaviors wouldn't work - and enough people will not read docs that they'll raise issues on it.

Sounds good to me.

@edgonmsft
Copy link
Contributor

Yep I like the name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Still under discussion, collecting feedback
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants