Bug 82089 - EasyHack: Remove lwpunoheader.hxx wrapper
Summary: EasyHack: Remove lwpunoheader.hxx wrapper
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
4.4.0.0.alpha0+ Master
Hardware: Other All
: medium normal
Assignee: cyrille.ruggero
URL:
Whiteboard: target:4.4.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: LWP
  Show dependency treegraph
 
Reported: 2014-08-03 12:33 UTC by Thomas Arnhold
Modified: 2017-09-24 19:19 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Here's a patch that should fix this easyhack. (3.42 KB, patch)
2014-08-04 12:08 UTC, cyrille.ruggero
Details
New attachment that follows Michael's recommandations. (16.00 KB, patch)
2014-08-15 12:32 UTC, cyrille.ruggero
Details
Remove th lwpunoheader.hxx wrapper. (8.15 KB, patch)
2014-10-15 07:29 UTC, cyrille.ruggero
Details
final attachment (6.04 KB, patch)
2014-10-18 09:55 UTC, cyrille.ruggero
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Arnhold 2014-08-03 12:33:04 UTC
There is a wrapper file lotuswordpro/source/filter/lwpunoheader.hxx. All files which include lwpunoheader.hxx should directly include the corresponding header files.

1. git grep 'lwpunoheader.hxx'
2. Fix all files
3. Remove lwpunoheader.hxx

Please leave this EasyHack for real beginners!!! If you are on, feel free to catch it.
Comment 1 cyrille.ruggero 2014-08-04 12:08:48 UTC
Created attachment 103989 [details]
Here's a patch that should fix this easyhack.

I'm a real beginner to open source contributing. I did what was said in the bug specification and my patch should fix this easyhack.
Comment 2 Michael Meeks 2014-08-15 02:00:25 UTC
Hi Cyrille, this is a great start - thanks for the patch :-)

Worth cleaning up a few things before we commit this though, I think:

It is good to have sal/config.h as the first include if possible.

Please remove the header you made redundant =)

Please filter any interface headers that are not used; just experiment with removing them in each module and seeing if cd lotuswordpro ; make - continues to work. 

When you've done that, doing a:

git commit -a

and attaching the result of git format-patch -1 as an here would make it much easier to merge (and includes the removal :-)

Many thanks for tackling this & looking forward to the next iteration.

All the best !
Comment 3 cyrille.ruggero 2014-08-15 12:32:10 UTC
Created attachment 104675 [details]
New attachment that follows Michael's recommandations.
Comment 4 cyrille.ruggero 2014-08-15 12:34:21 UTC
Sorry, I made a mistake with my post, so I needed to write a double post. I followed your recommandations Michael and I removed the redundant header.

I didn't find any unused interface headers.

Thank you for the advices, Michael.
Comment 5 Thomas Arnhold 2014-08-17 09:54:26 UTC
Hi Cyrille,

thanks for your work! Please could you create the patch with:

git format-patch -1

I can't read the current format. Thanks!
Comment 6 cyrille.ruggero 2014-10-15 07:29:22 UTC
Created attachment 107853 [details]
Remove th lwpunoheader.hxx wrapper.

This attachment should be compatible with libreoffice.
Comment 7 cyrille.ruggero 2014-10-15 07:31:33 UTC
Hi Thomas, this latest attachment follows your advices. Sorry for the two posts, I forgot that submitting an attachment created a new post.
Comment 8 Michael Meeks 2014-10-17 18:49:58 UTC
So - nice patch; but it -looks- like this bit is missing:

> Please filter any interface headers that are not used; just experiment
> with removing them in each module and seeing if cd lotuswordpro ;
> make - continues to work. 

By which I mean; that (almost certainly) we don't need -all- of these includes in each file; have you experimented with removing some of these lines to see if it fails ? ultimately, its possible that those are needed in other headers there, but ... the purpose was not just to remove this header, but remove redundant includes =)

Can you experiment with that ?

Thanks ...
Comment 9 cyrille.ruggero 2014-10-18 09:55:20 UTC
Created attachment 108025 [details]
final attachment

Oh, I understand, now it should be correct.
Comment 10 Michael Meeks 2014-10-18 18:27:14 UTC
Lovely; much better - see what a nice cleanup it is now =)
I've pushed to master - thanks for your contribution !

Please can you send a license statement to the mailing list of this form:

https://wiki.documentfoundation.org/Development/Developers#Example_Statement

Which helps our book-keeping; thanks !
Comment 11 Robinson Tryon (qubit) 2015-12-15 23:57:28 UTC Comment hidden (obsolete)