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.

Sign up for GitHub

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

Jump to bottom

updated logviewsettings #868

Merged
merged 5 commits into from
Jul 27, 2020
Merged

updated logviewsettings #868

merged 5 commits into from
Jul 27, 2020

Conversation

Barsonax
Copy link
Member

No description provided.

@ilexp ilexp mentioned this pull request Jul 14, 2020
Merged
@ilexp ilexp linked an issue Jul 14, 2020 that may be closed by this pull request
Refactor/rethink how duality handles settings #855
Open
@ilexp ilexp added this to the 4.0 milestone Jul 14, 2020
@ilexp ilexp added Editor Area: Duality editor or support libraries Plugin Area: Misc. core / editor plugins Task ToDo that's neither a Bug, nor a Feature labels Jul 14, 2020
@ilexp ilexp mentioned this pull request Jul 22, 2020
Open
@ilexp
Copy link
Member

ilexp commented Jul 22, 2020

Progress

ToDo

  • Discuss and tweak the draft. If we're happy with it, proceed:
  • Add XML docs for LogViewSettings class and members.
  • Add XML docs to all added API in LogView.
  • (Squash-)Merge PR and applying the same principle when addressing the others linked in Refactor/rethink how duality handles settings #855.

@ilexp
Copy link
Member

ilexp commented Jul 22, 2020

Ping @Barsonax, since I can't request a review from you.

@ilexp ilexp requested a review from a team July 22, 2020 10:51
@Barsonax
Copy link
Member Author

Looks good. One thing is that I see you merged master into this branch. Its cleaner to rebase this branch on master as that prevents the merge commit from being generated. Rebasing simply puts the new commits of this branch on top of master so you end up with a cleaner history.

ilexp
ilexp approved these changes Jul 24, 2020
View reviewed changes
Copy link
Member

@ilexp ilexp left a comment

Choose a reason for hiding this comment

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

Fixed the potential bug as mentioned in the comments, ready for (squash-)merge from my side.

@Barsonax
Copy link
Member Author

Can't approve since its my own PR so gonna to that by comment: Approved.

Merging this.

@Barsonax Barsonax merged commit 5794676 into AdamsLair:master Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilexp ilexp ilexp approved these changes

Assignees
No one assigned
Labels
Editor Area: Duality editor or support libraries Plugin Area: Misc. core / editor plugins Task ToDo that's neither a Bug, nor a Feature
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

Refactor/rethink how duality handles settings
2 participants
@Barsonax @ilexp