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

Expose brush dependencies, simplify BrushBuilder #4

Merged
merged 3 commits into from Apr 3, 2022

Conversation

Norlock
Copy link
Contributor

@Norlock Norlock commented Apr 3, 2022

Make important dependencies accessible for the end user. Like FontRef
for example, so a brush can be stored in a struct.

In my project I have a state struct so I can split a lot of the render code, but I can't use your library because dependencies are private. Anyway keep up the good work.

Make important dependencies accessible for the end user. Like FontRef
for example, so a brush can be stored in a struct.
@Blatko1
Copy link
Owner

Blatko1 commented Apr 3, 2022

What a coincidence! I just published v0.5.0 in which I worked on simmilar issues. Can you try to update your repo and commit the same changes and I'll merge.

@Blatko1 Blatko1 added the enhancement New feature or request label Apr 3, 2022
@Norlock
Copy link
Contributor Author

Norlock commented Apr 3, 2022

Ok I resolved conflicts, and added a type for readability. One tip I can give you is to maybe try to reduce generic usage, it can often make the code hard to read and is not always necessary. I haven't really looked at your code in debt, but for example the depth_stencil builder, I think you can simplify if its passed as a parameter in the same build function.

@Blatko1
Copy link
Owner

Blatko1 commented Apr 3, 2022

Thanks for the tip! I'll try to simplify things.

@Blatko1 Blatko1 merged commit 9dfedd7 into Blatko1:master Apr 3, 2022
@Blatko1 Blatko1 added the good first issue Good for newcomers label Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants