Achno / gowall

A tool to convert a Wallpaper's color scheme / palette
https://achno.github.io/gowall-docs/
MIT License
427 stars 8 forks source link

Better error handling and fixes for minor issues. #10

Closed elbachir-one closed 3 months ago

Achno commented 3 months ago

Wait don't change anymore stuff i will fully review things tomorrow or on Saturday since its an hour past midnight.

I do not like some changes ( the use of the continue keyword and the change there) and do you use a different formatter than the default one? it makes it difficult to see your actual changes and just different formatting of code without change.

I will continue the discussion tomorrow please do not touch business logic if you could or rearrange stuff , it makes it extremely hard to keep track of so many changes at different places and this PR is absolutely huge

elbachir-one commented 3 months ago

Got it, I'll pause any further changes for now. I used continue to skip processing for any theme wrapper (tw) that does not have a valid name or does not contain any colors. And skips the current color if it is invalid (hexToRGBA) function returns an error). This prevents invalid color values from being added to the theme's color list, to make sure that only valid colors are included.

Achno commented 3 months ago

Changes

Comments Code Review

if len(allThemes) == 0 {
    fmt.Println("No themes available.")
    return
}
switch  {
    case versionFlag: fmt.Printf("gowall version: %s\n",config.Version)

    default  : cmd.Help()
}

to

    if versionFlag {
            fmt.Printf("gowall version: %s\n", config.Version)
        } else {
            cmd.Help()

is a step backwards in my opinion, because when you want to insert new functionality you will enter if-else hell.

Thank you for your continuous contributions. This is certainly a learning experience for me and i hope its for you too :)

PS: Also please never create huge PR's with changes in 10+ files.