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

theme [nfc]: Make DesignVariables.{light,dark} final fields, not constructors #1236

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chrisbobbe
Copy link
Collaborator

Nicer to compute each of these just once.

@gnprice
Copy link
Member

gnprice commented Dec 30, 2024

(nit: the status quo is they're constructors, not methods)

@chrisbobbe chrisbobbe changed the title theme [nfc]: Make DesignVariables.{light,dark} final fields, not methods theme [nfc]: Make DesignVariables.{light,dark} final fields, not constructors Dec 30, 2024
@chrisbobbe
Copy link
Collaborator Author

Indeed :) thanks for the catch

Instead of constructors. Nicer to compute each of these just once.
@chrisbobbe chrisbobbe force-pushed the pr-design-variables-final-fields branch from aeb0f73 to 8cb19bb Compare December 30, 2024 20:06
@chrisbobbe chrisbobbe added the maintainer review PR ready for review by Zulip maintainers label Dec 30, 2024
@PIG208 PIG208 added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Jan 8, 2025
@PIG208
Copy link
Member

PIG208 commented Jan 8, 2025

LGTM! We should be able to apply this to MessageListTheme and etc., right?

@PIG208 PIG208 requested review from gnprice and removed request for PIG208 January 8, 2025 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants