Skip to content
This repository has been archived by the owner on Feb 25, 2024. It is now read-only.

refactor: improved code readibility and moved YaruVariant methods inside the enum #398

Closed

Conversation

TheLiux
Copy link
Contributor

@TheLiux TheLiux commented Oct 6, 2023

Hi
I made some changes to improve the code readability of the library.
I strongly believe that is better to use early returns instead of nested if.
Additionally, since Dart allows adding methods inside enums, I put inside the YaruVariant enum some methods that was scattered on the package some methods that return YaruVariant.

This will close the following issue: ubuntu/yaru.dart#851

Let me now what you think about it :D

@TheLiux TheLiux force-pushed the refactor/improved-code-readability branch from 3cfce6c to 75acf75 Compare October 6, 2023 10:35
@Feichtmeier
Copy link
Member

@TheLiux thank you

It would be better if you would split this PR into smaller ones. Let's start with the typo part please

@TheLiux TheLiux force-pushed the refactor/improved-code-readability branch from c7f6542 to 300f838 Compare November 14, 2023 08:44
@TheLiux
Copy link
Contributor Author

TheLiux commented Nov 14, 2023

@TheLiux thank you

It would be better if you would split this PR into smaller ones. Let's start with the typo part please

Hi, thanks for reply.
I removed the typo commit and added in another PR 😄
ubuntu/yaru.dart#402

@Feichtmeier
Copy link
Member

Feichtmeier commented Nov 14, 2023

Nice, ty :) !
Can you do the same with splitting YaruVariant stuff and the nested-if thing?
Our CI is not optimal and we still have the golden test PR pending (which we have working in yaru widgets), but we need to make sure that we dont end up with visual regressions
For the nested-if-PR please add screenshots showing that the theme stays the same
For the YaruVariant PR we need maybe a video where you toggle through the variants in gnome settings and see if a yaru app follows those changes (sorry for the additional work, but we are now at a point where a lot of canonical apps depend on this repo. We need to become better with CI/CD)

@TheLiux
Copy link
Contributor Author

TheLiux commented Nov 14, 2023

Nice, ty :) ! Can you do the same with splitting YaruVariant stuff and the nested-if thing? Our CI is not optimal and we still have the golden test PR pending (which we have working in yaru widgets), but we need to make sure that we dont end up with visual regressions For the nested-if-PR please add screenshots showing that the theme stays the same For the YaruVariant PR we need maybe a video where you toggle through the variants in gnome settings and see if a yaru app follows those changes (sorry for the additional work, but we are now at a point where a lot of canonical apps depend on this repo. We need to become better with CI/CD)

Yeah sure, I understand that!
I will do ASAP with the screenshots and media you need!
Just give me some time, i'll do it once I'm done with work 😅

@TheLiux TheLiux force-pushed the refactor/improved-code-readability branch 2 times, most recently from 5b820b4 to 6f3373f Compare November 14, 2023 11:20
@TheLiux TheLiux closed this Nov 14, 2023
@TheLiux TheLiux force-pushed the refactor/improved-code-readability branch from 6f3373f to dfc4fcb Compare November 14, 2023 11:21
@TheLiux TheLiux deleted the refactor/improved-code-readability branch November 14, 2023 11:21
@TheLiux
Copy link
Contributor Author

TheLiux commented Nov 14, 2023

I closed this PR and make new ones to have commit history aligned

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants