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

Make attributes display properly in dashboard #564

Merged
merged 1 commit into from
Jun 30, 2023

Conversation

YoonKiJin
Copy link
Contributor

@YoonKiJin YoonKiJin commented Jun 28, 2023

What this PR does / why we need it:

(Bug fix)

Which issue(s) this PR fixes:

Fixes yorkie-team/dashboard#128

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


Additional documentation:


Checklist:

  • Added relevant tests or not required
  • Didn't break anything

@CLAassistant
Copy link

CLAassistant commented Jun 28, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

Oh, you sent your first PR. 💪

Next, it would be good to do the below two things:

  1. Fix the error on the CI test
  2. Add related test code

@hackerwins hackerwins marked this pull request as draft June 28, 2023 06:10
@YoonKiJin YoonKiJin marked this pull request as ready for review June 29, 2023 09:18
@hackerwins hackerwins self-requested a review June 29, 2023 10:42
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

Thank you for reflecting on the requests.

Next, we need to remove unnecessary changes from this PR.
crdt/internal_document.go, database/database.go, memory/database.go

@YoonKiJin YoonKiJin closed this Jun 30, 2023
@YoonKiJin YoonKiJin force-pushed the main branch 2 times, most recently from bd60067 to abf17ac Compare June 30, 2023 02:25
… tests

Attributes should be displayed in dashboad JSON format data, and, because we fixed the JSON display for when attributes are added, we should write additional unit tests for them.
@YoonKiJin YoonKiJin reopened this Jun 30, 2023
@codecov
Copy link

codecov bot commented Jun 30, 2023

Codecov Report

Merging #564 (a2998fa) into main (abf17ac) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #564      +/-   ##
==========================================
- Coverage   51.94%   51.91%   -0.04%     
==========================================
  Files          67       67              
  Lines        6847     6851       +4     
==========================================
  Hits         3557     3557              
- Misses       2851     2855       +4     
  Partials      439      439              
Impacted Files Coverage Δ
pkg/document/crdt/tree.go 64.39% <0.00%> (-1.00%) ⬇️

@hackerwins hackerwins self-requested a review June 30, 2023 03:07
Copy link
Member

@hackerwins hackerwins left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@hackerwins hackerwins merged commit aa2f361 into yorkie-team:main Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug where attributes set in Tree are not displayed in the dashboard
3 participants