Make 'Rect' a 'constexpr' class#1909
Conversation
|
Maybe before commenting on We have guy who put in all the effort at the beginning of the year in #1741 and even split the commits by class. I know his PR might not be ready-to-use now, but I'm not too happy about telling him we still need to discuss the topic, and then later rewriting everything without him, with no single credit. This could make him reconsider future contributions, and rightfully so. What we could do is take his commits (under his author), perform the necessary changes and add |
Thanks for bringing this up, I do believe it is an important thing to consider. I'm totally happy for @fgallegosalido to take the credit for these changes, which are pretty much the same as the ones he contributed back in #1741. I am not sure what the easiest way to achieve that would be, but I also don't really feel like dealing with Is it possible to force-change the author of a commit? I'd be fine with changing the authorship of my commit to @fgallegosalido. Alternatively, if he is fine with it, I'd also propose simply adding "Originally contributed to @fgallegosalido" to these commits or something alongside those lines to avoid any Let me know what you think. |
|
An alternative would be a Maybe @fgallegosalido sees this himself and can tell us what he prefers? Otherwise I'd say we go with one of those two clauses (they are "official" in the sense that GitHub and other tools can parse them). |
|
Oh, no worries from my part. I'm totally fine with not being the final author of those commits. If you want to put me as co-author is fine, but I have no problem at all if you don't. I am already happy that you acknowledge my contributions, as I almost completely forgot about this matter. I think closing my pull request is fine and the one by @vittorioromeo should be accepted one, as it is more up-to-date. |
|
Thank you @fgallegosalido! @vittorioromeo: In that case, it's probably easiest if you leave this PR as-is and simply change the commit message to include: I'll try to review the technical parts later today 🙂 |
66421d6 to
781a364
Compare
|
Added! :) |
|
I also think we can close #1741, as these wave of changes are much more complete than what I did back then. Thank you @vittorioromeo for your contributions! We all wanted SFML to be modernized ^.^ |
781a364 to
1e3ff13
Compare
|
@Bromeon: ping :) |
| // Not using 'std::min' and 'std::max' to avoid depending on '<algorithm>' | ||
| const auto min = [](T a, T b){ return (a < b) ? a : b; }; | ||
| const auto max = [](T a, T b){ return (a < b) ? b : a; }; |
There was a problem hiding this comment.
lol (not upset, just having a laugh)
Description
This PR turns
sf::Rectinto aconstexpr-friendly class, and also provides a few more improvements:<algorithm>from a widely used header, improving compilation timeRect.hpp, so they have been fixedsf::qualification when referring toVectorinsideRectTasks
How to test this PR?
CI.