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

[blend2d] Update to beta_2020-05-04 #11155

Merged
merged 1 commit into from
May 8, 2020
Merged

[blend2d] Update to beta_2020-05-04 #11155

merged 1 commit into from
May 8, 2020

Conversation

ZeeWanderer
Copy link
Contributor

initial impl of font manager is here

@LilyWangL LilyWangL requested a review from NancyLi1013 May 6, 2020 02:00
@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines requires:testing Needs tests added before merging and removed info:reviewed Pull Request changes follow basic guidelines labels May 6, 2020
@NancyLi1013
Copy link
Contributor

NancyLi1013 commented May 6, 2020 •
edited
Loading

Hi @gnu-lorien
LGTM, thanks for this PR.

Since all features are set as default features, it's not necessary to test them now.

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:testing Needs tests added before merging labels May 6, 2020
@ZeeWanderer
Copy link
Contributor Author

ZeeWanderer commented May 6, 2020 •
edited
Loading

Would it be better to change features to no_{feature_name} to avoid having most or all features as default? Having features that disable functional seems counter-intuitive. The current set of default features mirror default CMake behavior when built without any additional options.

@BillyONeal
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil
Copy link
Contributor

@ZeeWanderer no, it would not be; features should be purely additive.

On the other hand, thank you so much! This looks great :D

@strega-nil strega-nil merged commit 124e815 into microsoft:master May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NancyLi1013 NancyLi1013 NancyLi1013 approved these changes

Assignees

@NancyLi1013 NancyLi1013

Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ZeeWanderer @NancyLi1013 @BillyONeal @strega-nil