Review Pls about ico-builder HOT 5 CLOSED

Aaah I see, fair enough. Consider the review finished in that case :)

from ico-builder.

@janhohenheim

from ico-builder.

• Derive Deref and DerefMut for IconSizes and remove the .0 from sizes.0
• IMO these lines violate the SRP and should be in their own function
• Replace the PathBuf::push call with let output_path: PathBuf = [out_dir, file_name].iter().collect();, see the docs
• Replace the .unwrap calls with .expect with a nice message
• Rename buf to buffer
• Move the example in the docs to an actual example
• What happens when the input images are not squares?
• Reduce code duplication by making add_source_file call add_source_files
• IconSizes::new requires 'static input, limiting its usefulness in e.g. GUIs with user input. It uses a Cow internally, so this is unnecessary

from ico-builder.

Thaaanks so much for your review ❤️. You indeed caught some things I have missed :)

I have addressed most of the comments and checked them off the list.

Derive Deref and DerefMut for IconSizes and remove the .0 from sizes.0.

I can only implement Deref as my underlying Cow might be a non-mutable reference :)

• IconSizes::new requires 'static input, limiting its usefulness in e.g. GUIs with user input. It uses a Cow internally, so this is unnecessary

I only need the Cow so that I can create IconSizes in const contexts. In that case 'static is indeed required.

If you have non-static data, you can use this From impl:

impl<'a, I> From<I> for IconSizes
where
    I: IntoIterator<Item = &'a u32>

from ico-builder.

Oh, I forgot to close this :) Thanks again for the review ❤

from ico-builder.