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

Controls: Remove ArrayControl #15788

Merged
merged 3 commits into from Aug 8, 2021
Merged

Controls: Remove ArrayControl #15788

merged 3 commits into from Aug 8, 2021

Conversation

jonspalmer
Copy link
Contributor

What I did

  • Removed ArrayControl and supporting types
  • Removed Server support for Array Control types

The ArrayControl was replace with the ObjectControl in 6.2 - f0cf8bab. This cleans up the now unused code.

@jonspalmer jonspalmer added maintenance User-facing maintenance tasks server addon: controls labels Aug 7, 2021
@jonspalmer jonspalmer requested a review from shilman August 7, 2021 17:55
@nx-cloud
Copy link

nx-cloud bot commented Aug 7, 2021

Nx Cloud Report

CI ran the following commands for commit f011a67. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@@ -80,7 +75,6 @@ export type ControlType =
| 'text';

export type Control =
| ArrayConfig
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suspect that technically this is a breaking change. Consumers could have been passing a separator param which has been ignored since we switched to ObjectControl. Do we want a deprecation warning around this or just do it?

Copy link
Member

Choose a reason for hiding this comment

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

KISS

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

LGTM! great catch 🚀

@@ -80,7 +75,6 @@ export type ControlType =
| 'text';

export type Control =
| ArrayConfig
Copy link
Member

Choose a reason for hiding this comment

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

KISS

@shilman shilman added this to the 6.4 PRs milestone Aug 8, 2021
@shilman shilman merged commit f9f3256 into next Aug 8, 2021
@shilman shilman deleted the remove_array_control branch August 8, 2021 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: controls maintenance User-facing maintenance tasks server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants