Skip to content
This repository was archived by the owner on Nov 16, 2023. It is now read-only.

feat: remove prebuilt entity types options#1269

Open
mattmazzola wants to merge 4 commits intomasterfrom
mattm/remove-prebuilts
Open

feat: remove prebuilt entity types options#1269
mattmazzola wants to merge 4 commits intomasterfrom
mattm/remove-prebuilts

Conversation

@mattmazzola
Copy link

Only removes from dropdown for now. Will need to work on server after. There was a lot of complexity with overloading the entity type to be custom string and relying on prebuilt prefix stuff. Perhaps we can remove a lot of logic there.

I assume we'll have to leave it in for a while but the CLM.isPrebuilt function is now obsolete since there can't be prebuilts, only entities with resolver.

Before:
image

After:
image

Fixes ADO#2232 (Partially)

@mattmazzola mattmazzola requested a review from LarsLiden August 26, 2019 18:51
@mattmazzola
Copy link
Author

After more investigation, we had to leave the extra EntityTypes when editing Entity for existing bots which used direct prebuilts. Perhaps we can remove them in few months after people migrate.

Also, was looking at cleaning up server also but the prebuilts that are implicitly created as a result of creating entity with resolver also exist at the top level so they still need the EntityTypes defined there.

@mattmazzola mattmazzola requested a review from tpmsr August 27, 2019 16:01
Copy link
Member

@LarsLiden LarsLiden left a comment

Choose a reason for hiding this comment

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

Can we discuss this at the design review meeting. I seem to remember that someone (Shahin?) thought there was a use case where folks might still need this.

@mattmazzola mattmazzola added FYI Low Priority PR - Can be merged without review. needs-review High priority PR, Needs code review before merge and removed FYI Low Priority PR - Can be merged without review. labels Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

needs-review High priority PR, Needs code review before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants