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: Remove all old frontend parsing for nested columns #1919

Merged
merged 1 commit into from
Jul 1, 2022

Conversation

kristenarmes
Copy link
Contributor

Signed-off-by: Kristen Armes karmes@lyft.com

Summary of Changes

This change removes all the code related to the previous implementation of nested columns that used frontend parsing. This helps to avoid having two unrelated implementations of the same feature.

Things to note:

  • If you are setting the nestedColumns.isEnabled config, that has been removed from the default config and config types since it is no longer used, so it should be removed from your configuration as well.
  • If you are interested in having the new nested column display, refer to the details of how to configure the ComplexTypeTransformer. This will set up the required backend data to display complex nested types on the frontend.

Tests

Removed any test that were no longer relevant

Documentation

N/A

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • PR includes a summary of changes.
  • PR adds unit tests, updates existing unit tests, OR documents why no test additions or modifications are needed.
  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does

Signed-off-by: Kristen Armes <karmes@lyft.com>
@boring-cyborg boring-cyborg bot added area:frontend From the Frontend folder category:api labels Jul 1, 2022
@kristenarmes kristenarmes marked this pull request as ready for review July 1, 2022 20:00
@@ -385,7 +385,6 @@ const configDefault: AppConfig = {
isEnabled: false,
},
nestedColumns: {
isEnabled: true,
Copy link
Member

Choose a reason for hiding this comment

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

Are there any documentation that we need to update due to the removal of this option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is not, I referred to the original PRs for the addition of these changes to make sure I covered all the parts to be removed, and didn't come across any related docs

Copy link
Member

@Golodhros Golodhros left a comment

Choose a reason for hiding this comment

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

LGTM

@kristenarmes kristenarmes merged commit 50f4193 into amundsen-io:main Jul 1, 2022
@kristenarmes kristenarmes deleted the remove-old-fe-parsing branch July 1, 2022 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:frontend From the Frontend folder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants