Re: [Swig-devel] Octave patches

[Karl W. <kar...@gm...>]

Hi William,

I apologise if my recent changes have not been up to the required
standard. I think I did try to use the existing make rules in SWIG,
but I believe they use indent which doesn't seem to always work well
with C++ code, hence my trying to use astyle. However it seems I
haven't got the style right.

If you're unhappy with my changes, it would probably be best to revert
them using "git revert" - I imagine that's the best that can be done.
I can then try to improve them before attempting to re-commit them - I
will in future use a pull request so they can be reviewed first.

Cheers, Karl


On 7 October 2014 23:40, William S Fulton <ws...@fu...> wrote:
>
> Hi Karl,
>
> I'm glad you sorted out the Octave build failures, I wasn't sure what it was
> nor able to come up with a solution. The 20 minute run time is about right
> as that is roughly what the other languages take.
>
> I guess dropping support for versions < 3.2.0 is okay if it is going to
> really help maintenance in the future. I didn't notice anything in
> configure.ac that turns off testing if the version is not new enough. Can
> you implement that please, else 'make check' will fail unnecessarily on old
> platforms.
>
> Regarding your recent patches 5ac475cbcd873307abe802063789f47aa5435dd1 and
> bf12d76d037dddb68c294cd77c3ad1f6b6d2e985, I'm not very impressed. They mix
> white space changes with code changes. Never ever do that please. It makes
> it really hard to see the real code changes. 'git blame' is now also pretty
> useless. Secondly the white space change is wrong, eg it uses 'char*'
> instead of the SWIG coding conventions of 'char *'. What on earth made you
> make this change? The command to beautify a file is documented in
> Extending.html. Although this might require a few manual fixes afterwards,
> this is what you should have been using.
>
> I'd like to see an improvement to the current approach to ensure consistent
> formatting, such as migrating to astyle. I tried astyle a few years ago, but
> couldn't get it to work very well. If it is better now, I'd be interested to
> know.
>
> I would really like to move HEAD back to before your commits, but I don't
> know how this can be done in Git without breaking the repo for everyone as
> you have made your changes on master and made it public. Does anyone know?
>
> William
>
>
> On 05/10/14 15:14, Karl Wette wrote:
>>
>> I've also tried to fix the occasional Travis failures for Octave,
>> which I believe were due to gcc running out of memory when compiling
>> the Octave tests. I've pushed a patch which disables all optimisations
>> when compiling the Octave tests, which should result in faster
>> compiles and less memory usage. I then reduced the number of make jobs
>> in .travis.yml from 4 to 3 to give more memory per job - my own Travis
>> build still has the Octave test completing in under 20min, due in part
>> to the speedup from removing optimisations. Finally I've reverted
>> William's "Suppress Octave failing the build" patch.
>>
>> Of course if the Octave build starts failing again, please reapply
>> this patch and let me know.
>>
>> Cheers, Karl
>>
>>
>> On 4 October 2014 20:29, Karl Wette<kar...@gm...>  wrote:
>>>
>>> FYI, I've just pushed a number of patches to the Octave module. In
>>> particular, Octave versions older than 3.2.0 are no longer supported -
>>> 3.2.0 itself is 5 years old, and I'm not aware of any still-supported
>>> distribution using an older version. This allows some internal cleanup
>>> of the Octave module, and makes it easier to continue testing SWIG
>>> against more modern versions of Octave. Any objections, please let me
>>> know.
>>>
>>> Cheers, Karl
>>
>>
>> ------------------------------------------------------------------------------
>> Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
>> Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
>> Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
>> Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
>>
>> http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Swig-devel mailing list
>> Swi...@li...
>> https://lists.sourceforge.net/lists/listinfo/swig-devel
>
>