Re: Review Request 129742: Add option FORCE_ENABLE_HUNSPELL

2017-01-15 Thread Martin Tobias Holmedahl Sandsmark


> On Jan. 15, 2017, 12:46 p.m., Martin Tobias Holmedahl Sandsmark wrote:
> > Ship It!

tbh., I have no idea why it uses the explicit versions, pre-1.2 is ancient and 
I doubt anyone uses it so I don't think even a minimum version check is 
necessary.

I see at least on arch it is symlinked to just libhunspell.so, so I made it 
look for just plain libhunspell.so as well, but feel free to commit this as 
well (from packages.debian.org it doesn't look like debian does the same 
symlink).


- Martin Tobias Holmedahl


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129742/#review102038
---


On Jan. 1, 2017, 11:01 p.m., Andreas Sturmlechner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129742/
> ---
> 
> (Updated Jan. 1, 2017, 11:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: sonnet
> 
> 
> Description
> ---
> 
> HUNSPELL is optional, but its cmake module depends on updates on each
> future hunspell release to be found. Each time that happens, the user
> build is potentially silently missing a feature, because up to now it
> can not be reliably enabled, only disabled.
> 
> This was the least disruptive solution I could come up with.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt edac12f6cdc1fddc7f6b9df0baeb519a6c6502fb 
>   src/plugins/CMakeLists.txt 58b254c670e7e57202bbc9c889c86a48041a044e 
> 
> Diff: https://git.reviewboard.kde.org/r/129742/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sturmlechner
> 
>



Re: Review Request 129742: Add option FORCE_ENABLE_HUNSPELL

2017-01-15 Thread Martin Tobias Holmedahl Sandsmark

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129742/#review102038
---


Ship it!




Ship It!

- Martin Tobias Holmedahl Sandsmark


On Jan. 1, 2017, 11:01 p.m., Andreas Sturmlechner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129742/
> ---
> 
> (Updated Jan. 1, 2017, 11:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: sonnet
> 
> 
> Description
> ---
> 
> HUNSPELL is optional, but its cmake module depends on updates on each
> future hunspell release to be found. Each time that happens, the user
> build is potentially silently missing a feature, because up to now it
> can not be reliably enabled, only disabled.
> 
> This was the least disruptive solution I could come up with.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt edac12f6cdc1fddc7f6b9df0baeb519a6c6502fb 
>   src/plugins/CMakeLists.txt 58b254c670e7e57202bbc9c889c86a48041a044e 
> 
> Diff: https://git.reviewboard.kde.org/r/129742/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sturmlechner
> 
>



Re: Review Request 129742: Add option FORCE_ENABLE_HUNSPELL

2017-01-02 Thread Albert Astals Cid


> On Jan. 2, 2017, 10:53 p.m., Albert Astals Cid wrote:
> > I don't think this makes any sense. I understand your pain, would i'd 
> > rather improve the search of hunspell than accept it's crap and you want 
> > people to override it.
> 
> Andreas Sturmlechner wrote:
> It sure ain't pretty. But then I don't know why the hunspell search is 
> done how it's done now. Is it just for lack of being able to set a minimum 
> version?

No idea to be honest, but personally i'd be much happier with a patch that sets 
a minimum version than this "i don't care let me through" one. Martin may feel 
differently, so let's wait for him to say something.


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129742/#review101749
---


On Jan. 1, 2017, 11:01 p.m., Andreas Sturmlechner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129742/
> ---
> 
> (Updated Jan. 1, 2017, 11:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: sonnet
> 
> 
> Description
> ---
> 
> HUNSPELL is optional, but its cmake module depends on updates on each
> future hunspell release to be found. Each time that happens, the user
> build is potentially silently missing a feature, because up to now it
> can not be reliably enabled, only disabled.
> 
> This was the least disruptive solution I could come up with.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt edac12f6cdc1fddc7f6b9df0baeb519a6c6502fb 
>   src/plugins/CMakeLists.txt 58b254c670e7e57202bbc9c889c86a48041a044e 
> 
> Diff: https://git.reviewboard.kde.org/r/129742/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sturmlechner
> 
>



Re: Review Request 129742: Add option FORCE_ENABLE_HUNSPELL

2017-01-02 Thread Andreas Sturmlechner


> On Jan. 2, 2017, 10:53 p.m., Albert Astals Cid wrote:
> > I don't think this makes any sense. I understand your pain, would i'd 
> > rather improve the search of hunspell than accept it's crap and you want 
> > people to override it.

It sure ain't pretty. But then I don't know why the hunspell search is done how 
it's done now. Is it just for lack of being able to set a minimum version?


- Andreas


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129742/#review101749
---


On Jan. 1, 2017, 11:01 p.m., Andreas Sturmlechner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129742/
> ---
> 
> (Updated Jan. 1, 2017, 11:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: sonnet
> 
> 
> Description
> ---
> 
> HUNSPELL is optional, but its cmake module depends on updates on each
> future hunspell release to be found. Each time that happens, the user
> build is potentially silently missing a feature, because up to now it
> can not be reliably enabled, only disabled.
> 
> This was the least disruptive solution I could come up with.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt edac12f6cdc1fddc7f6b9df0baeb519a6c6502fb 
>   src/plugins/CMakeLists.txt 58b254c670e7e57202bbc9c889c86a48041a044e 
> 
> Diff: https://git.reviewboard.kde.org/r/129742/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sturmlechner
> 
>



Re: Review Request 129742: Add option FORCE_ENABLE_HUNSPELL

2017-01-02 Thread Albert Astals Cid

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129742/#review101749
---



I don't think this makes any sense. I understand your pain, would i'd rather 
improve the search of hunspell than accept it's crap and you want people to 
override it.

- Albert Astals Cid


On Jan. 1, 2017, 11:01 p.m., Andreas Sturmlechner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129742/
> ---
> 
> (Updated Jan. 1, 2017, 11:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.
> 
> 
> Repository: sonnet
> 
> 
> Description
> ---
> 
> HUNSPELL is optional, but its cmake module depends on updates on each
> future hunspell release to be found. Each time that happens, the user
> build is potentially silently missing a feature, because up to now it
> can not be reliably enabled, only disabled.
> 
> This was the least disruptive solution I could come up with.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt edac12f6cdc1fddc7f6b9df0baeb519a6c6502fb 
>   src/plugins/CMakeLists.txt 58b254c670e7e57202bbc9c889c86a48041a044e 
> 
> Diff: https://git.reviewboard.kde.org/r/129742/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sturmlechner
> 
>



Review Request 129742: Add option FORCE_ENABLE_HUNSPELL

2017-01-01 Thread Andreas Sturmlechner

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129742/
---

Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.


Repository: sonnet


Description
---

HUNSPELL is optional, but its cmake module depends on updates on each
future hunspell release to be found. Each time that happens, the user
build is potentially silently missing a feature, because up to now it
can not be reliably enabled, only disabled.

This was the least disruptive solution I could come up with.


Diffs
-

  CMakeLists.txt edac12f6cdc1fddc7f6b9df0baeb519a6c6502fb 
  src/plugins/CMakeLists.txt 58b254c670e7e57202bbc9c889c86a48041a044e 

Diff: https://git.reviewboard.kde.org/r/129742/diff/


Testing
---


Thanks,

Andreas Sturmlechner