← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/bug-986611-cppcheck_performance into lp:widelands

 

Review: Approve

minor nit, otherwise lgtm. 

I am not convinced these changes are a terrible good investment of time though - they might introduce new bugs and performance changes of these changes will not impact the game noticeably at all. What do you think?

Diff comments:

> 
> === modified file 'src/graphic/text_parser.cc'
> --- src/graphic/text_parser.cc	2016-08-04 15:49:05 +0000
> +++ src/graphic/text_parser.cc	2016-08-04 17:16:45 +0000
> @@ -47,15 +47,15 @@
>  	text_align_ = src.text_align_;
>  }
>  
> -TextBlock::TextBlock() {
> -	font_size_ = 10;
> -	font_color_ = RGBColor(255, 255, 0);
> -	font_weight_ = "normal";
> -	font_style_ = "normal";
> -	font_decoration_ = "none";
> -	font_face_ = (UI::g_fh1->fontset())->sans();
> -	line_spacing_ = 0;
> -}
> +TextBlock::TextBlock() :

formatting here is weird - are we still using tabs for indentation? I thought we ran clan-format to get rid of this?

> +   font_size_(10),
> +	font_color_(RGBColor(255, 255, 0)),
> +	font_weight_("normal"),
> +	font_style_("normal"),
> +	font_decoration_("none"),
> +	font_face_((UI::g_fh1->fontset())->sans()),
> +	line_spacing_(0)
> +{}
>  
>  void TextParser::parse(std::string& text, std::vector<RichtextBlock>& blocks) {
>  	bool more_richtext_blocks = true;


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-986611-cppcheck_performance/+merge/301043
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-986611-cppcheck_performance.


References