search

BoostGSoC21 / math

Boost.org math module
http://boost.org/libs/math
Boost Software License 1.0
0 stars 1 forks source link

Clean up #28

Closed Lagrang3 closed 3 years ago

Lagrang3 commented 3 years ago

This PR consists of code cleanup.

ckormanyos commented 3 years ago

Hi @Lagrang3 I could not solve this particular problem in the configuration available.

I was stumped by this problem and will address it with clear advice.

I reverted a few changes and got CI running again. I found that you/we had either now or sooner (not sure when) added even more link dependencies such as pmr to some test files. I advise against adding these sorts of link dependencies, as we would like CI to rely on as little as possible.

Also, I encourage us to be wise and avoid breaking CI so close to the deadline. I can accept a few tweaks and changes to push forward and improve. But leaving a hanging branch with broken CI does not make us look very strong and good. It leaves the wrong impression about the quality of our/your work. Appearance does matter a bit at this stage. In summary, I strongly encourage clean branches, absolutely a clean develop branch running CI and low risk taking prior to deadline.

You have done great work --- keep it that way.

@cosurgi

Lagrang3 commented 3 years ago

@ckormanyos. I understand. In fact, to me the changes seemed quite innocent and some where bug fixes. In any case PR are not compulsory to merge. You're right we are too close to the deadline. We could stop at this point either by merging this or not. And in the future we will finish the unfinished.

ckormanyos commented 3 years ago

We could stop at this point either by merging this or not. And in the future we will finish the unfinished.

Good choice Eduardo. OK @Lagrang3 I've merged and we will proceed according to your words...