Adding static const rank to allow sfinae based on rank#2138
Adding static const rank to allow sfinae based on rank#2138tdegeus merged 1 commit intoxtensor-stack:masterfrom
Conversation
JohanMabille
left a comment
There was a problem hiding this comment.
Thanks, the PR is really nice. I has a nitpicking remark regarding the definition.
We may want to add the rank member to other expressions, based on the rank of underlying expressions, but that can be done in a dedicated PR (might need additional metafunctions).
Also could you squash a bit (maybe one commit for the implementation and one for the documentation). I can do it with the UI but this prevents adding a merge commit, which is annoying when browsing the history to write the changelog.
|
Thanks @JohanMabille . It would be good to list where we would like to add |
|
The issue with squashing on Github is that it does not create a merge commit with a reference to the PR. So it's more complicated (and time-consuming) to track for writing the changelog before a release. |
|
@JohanMabille There is a problem appearing in the CI with |
|
Indeed, |
|
Also there are some windows errors for |
|
That's really weird :s Looks more like a compiler issue (otherwise other builds would fail). I need to dig a bit, I don't have this part of the code in mind. Regarding the |
|
It could indeed be a compiler bug. Shall I just comment the test here and squash so that we can merge. And then open a PR uncommenting the tests, such that we can quietly debug? |
|
Yes it would be nice to have this one merged quickly. |
|
Good to go! Will open a PR once merged. A separate question: I guess I would have to add the member is |
|
If you want to be able to use the |
|
😇 |
Checklist
Description
Fixes #2135