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

Remove legacy JSON code #1356

Merged
merged 11 commits into from Feb 8, 2021
Merged

Conversation

dominiklohmann
Copy link
Member

@dominiklohmann dominiklohmann commented Feb 8, 2021

📔 Description

This PR fixes up @ngrodzitski's work in #1343 so we can merge it into master.

📝 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

🎯 Review Instructions

Commit-by-commit.

@dominiklohmann dominiklohmann added the refactoring Restructuring of existing code label Feb 8, 2021
@dominiklohmann dominiklohmann marked this pull request as ready for review February 8, 2021 14:26
Copy link
Member

@tobim tobim left a comment

Choose a reason for hiding this comment

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

All good as far as I can tell.
edit: I did test the output of vast version and vast status. The other stuff should be covered by CI.

@dominiklohmann
Copy link
Member Author

I did test the output of vast version and vast status. The other stuff should be covered by CI.

They are both covered by integration tests. The example plugin integration test checks vast version , and vast status we cover in multiple places.

@dominiklohmann dominiklohmann merged commit 18f1128 into master Feb 8, 2021
@dominiklohmann dominiklohmann deleted the story/ch4981/legacy-json-removal branch February 8, 2021 14:49
@@ -13,6 +13,20 @@ This changelog documents all notable user-facing changes of VAST.

## Unreleased

### ⚡️ Breaking Changes
Copy link
Member

Choose a reason for hiding this comment

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

Uh, was this added intentionally?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, that was definitely a mistake. Can you remove this in your other PR that's likely next to be merged?

Copy link
Member

Choose a reason for hiding this comment

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

The one for the partition_selector is probably next, but that doesn't touch the changelog. I'll remove it in the PR that moves around the options.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!

tobim added a commit that referenced this pull request Feb 17, 2021
This reverts commit 18f1128, reversing
changes made to 9fd9da5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Restructuring of existing code
Projects
None yet
3 participants