Skip to content

Allowing Container's background and border to be removed.#1558

Merged
cmyr merged 3 commits intolinebender:masterfrom
sjoshid:Container_fix
Jan 31, 2021
Merged

Allowing Container's background and border to be removed.#1558
cmyr merged 3 commits intolinebender:masterfrom
sjoshid:Container_fix

Conversation

@sjoshid
Copy link
Contributor

@sjoshid sjoshid commented Jan 29, 2021

Closes #1554

Copy link
Contributor

@tirix tirix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically nothing to say!
I only have a remark on the naming.

}

/// Removes background.
pub fn remove_background(&mut self) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm used to the 'set/clear' and 'add/remove' verbs used in pairs. This would be set/remove, so would the right verb rather be 'clear_background'?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Made the change.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, I would go with clear here. otherwise looks good!

@cmyr cmyr merged commit 393db3e into linebender:master Jan 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow container's background and border to be removed

3 participants