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

refactor(artifacts): consolidate artifacts feature flags checks #8096

Merged
merged 3 commits into from
Mar 26, 2020

Conversation

maggieneterval
Copy link
Contributor

@maggieneterval maggieneterval commented Mar 25, 2020

Related to: spinnaker/governance#111

The artifacts and artifactsRewrite flags were intended to be mutually exclusive, but it is possible that this was not fully thought through, as there are several workflows (Find Artifact from Resource stage, Find Artifact from Execution stage) that currently require both flags to be enabled in order to use with the new UI. This PR addresses that bug and consolidates the artifacts feature flag checks to a single service.

  • refactor(artifacts): consolidate artifacts feature flags checks

    Adds ArtifactsModeService, which is responsible for the mapping between the four possible feature flag configuration states and the three possible UI experiences. Replaces each instance of logic conditional on one or both artifacts feature flags with logic conditional on the UI experience to which they map.

  • fix(artifacts): enable artifact-specific stages when only artifactsRewrite feature flag is enabled

    Register the Find Artifact from Resource stage and Find Artifact from Execution stage if at least one of the two existing artifacts feature flags is enabled.

Pending a successful audit of the STANDARD UI experience, we will:

  • Remove the DISABLED experience (1.20).
  • Remove the artifacts and artifactsRewrite feature flags (1.20).
  • Default to the STANDARD experience unless a new, temporary legacyArtifacts flag is enabled (1.20).
  • Remove the LEGACY experience and legacyArtifacts flag. With only the STANDARD experience remaining, we can remove the ArtifactsModeService and all conditional calling code (1.21).

Copy link
Contributor

@ezimanyi ezimanyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏺 🎏 ✔️🏺 🎏 ✔️🏺 🎏 ✔️🏺 🎏 ✔️
⬇️
🏺 🎏 ✔️
You have greatly simplified things!

@maggieneterval maggieneterval added the ready to merge Reviewed and ready for merge label Mar 26, 2020
@mergify mergify bot merged commit 51dba01 into spinnaker:master Mar 26, 2020
yunzhangit pushed a commit to yunzhangit/deck that referenced this pull request Mar 28, 2021
…naker#8096)

* refactor(artifacts): consolidate artifacts feature flags checks

* fix(artifacts): enable artifact-specific stages when only `artifactsRewrite` feature flag is enabled

* fix(artifacts): make artifactsMode read-only
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Reviewed and ready for merge target-release/1.20
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants