Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SplineBuilder doc #409
SplineBuilder doc #409
Changes from 97 commits
f60b990
84a5ca7
5b67b46
1f02beb
96d4fdb
b9c4451
077807a
373e034
653bab4
889b9f9
20ca22d
6238ca3
6ab4a97
64659a7
66ea164
2fdb975
93ec4a0
29367d7
97a042e
84e76df
aa7b594
3ea940f
ffef0b2
2a050b0
880353b
39f2d70
c0ef458
56dbf4e
85a2a0f
17bb0ba
7e6c05b
8c3bb44
d7472c7
d98abe5
a563f6d
7017234
dcc5c6f
8f7f760
9d675c9
51ffa31
2d0737c
6850ae9
937d432
9ffbe04
19b1c1e
8c5c579
0589694
ffe161e
1649b9a
f853bca
fb64952
7a87e97
ccc2163
65c221e
49b9b01
195dc50
bae41cc
e702a5b
f31ab4a
f180f4d
1484581
90ccf09
792b7f1
e516124
a0baf93
d5f56b5
29a9c3c
bfa7316
f557230
e3133e5
a0dc609
403a99a
bdd529a
8068c3b
ad55555
45db898
3e9c720
5f82177
6f3d65c
7dfbed2
669719e
0e27bfe
3982b95
9e5c166
e302ef9
b3f4724
d6890d9
995a5df
167a36f
19e3cc9
bd7420c
12d32d8
a268a08
74da282
a972961
c116d00
b247fbc
185ca11
89bff3f
bdc3ac8
515dc8d
0a34fca
127a22b
948259f
d734870
68b639e
095635e
dc4d3f6
9273432
0ddd898
eef77b9
d61deeb
932620e
b3d9c54
bc8ce54
023e90c
bf1d586
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of these lines are quite long. Could they be broken up. E.g:
This makes it easier to read these docs on a small screen or while opening 2 files side by side.
I'm not sure what the recommended maximum line length is that clang-format uses. But sticking to something similar should be preferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be done throughout the file (e.g. around line 176). If you don't usually think about this, then probably it is easiest to complete this last
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm I think this is done throughout the file already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look like it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang's default maximum line length is 80 characters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The longest line in the screenshot is 189 characters long.
I'm not too worried about the lines that are 100 characters long but more than double is definitely too long
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it
x
that is "moved" ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to the SplineBuilder once moved
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tpadioleau I think this is the last remaining comment here, to me something can be qualified as "moved" only once it has been moved, and only the moved thing not the remaining empty place where it comes from. But this is a bit too philisophical to me so please suggest something if you don't agree ^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep the original doc on the return section. The reference is really on the object
*this
, not the object that has been moved,x
.