-
Notifications
You must be signed in to change notification settings - Fork 769
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
[atomics.ref.ops], [atomics.types.operations], [util.smartptr.atomic.… #6518
Conversation
4227449
to
ad964de
Compare
I need some advice how to fix the "overfull \hbox" errors. It seems to say that the resulting text would need too much space in horizontal direction. OK, let's look at the first case "lines 3154--3160": How is it possible to edit the text to make it even smaller? |
For reviewing, it's not helpful that the PDF isn't uploaded because the output check failed. |
I suspect this is not something I can fix, right? Regarding my text changes: I tried to follow the pre-existing line-break structure. Maybe I shouldn't and I should try to provide each paragraph as filled line with breaks at around 70 characters? |
If you built it locally, you can look at which syllable a word overfills. |
Hmmh, I'm on Windows and I'm not sure how to setup my infrastructure to create the pdf locally. If there is no other way I need to experiment with my PULL request. |
I don't think you need to, as it is a separate problem.
The source line breaks are independent of the rendered text.
which I wish was more strictly applied. |
To get the PDF uploaded, |
But isn't that a setting that should be independently set from my PULL request? |
4f35db3
to
cdd3338
Compare
source/threads.tex
Outdated
@@ -3146,8 +3144,8 @@ | |||
\begin{itemdescr} | |||
\pnum | |||
\expects | |||
\tcode{order} is | |||
neither \tcode{memory_order::release} nor \tcode{memory_order::acq_rel}. | |||
\tcode{order} is either \tcode{memory_order::relaxed}, \tcode{memory_order::consume}, |
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.
Can we harmonize this to read "The order argument is ..." (see next change)
Or maybe the other way round.
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 harmonized it now as of your first suggestion.
No. In general, I think we can expect someone editing the draft to be able to build locally before uploading. And we only want "good" artifacts published from our continuous integration pipeline. |
cdd3338
to
2b6150c
Compare
source/threads.tex
Outdated
\tcode{memory_order::consume}, | ||
\tcode{memory_order::acquire}, nor | ||
\tcode{memory_order::acq_rel}. | ||
The \tcode{order} argument is either \tcode{memory_order::relaxed}, |
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.
Strictly speaking, "order" is not an argument, but the name of a parameter.
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.
So do you wish me to replace "argument" by "parameter"?
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 decided to perform this replacement
source/threads.tex
Outdated
\tcode{memory_order::consume}, | ||
\tcode{memory_order::acquire}, nor | ||
\tcode{memory_order::acq_rel}. | ||
The \tcode{order} argument is either \tcode{memory_order::relaxed}, |
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 believe "either" in English preferentially refers to a choice of two things. We have three things here.
I'd suggest to simply drop "either"; I don't think it adds anything.
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.
OK.
…shared], [util.smartptr.atomic.weak], [atomics.flag]: Reword Preconditions on memory_order values in a positive form and harmonize wording style, also replace 'argument' by 'parameter'.
2b6150c
to
6467e1e
Compare
I've redone the patch: #6519 It keeps the semantic linebreaks and removes "The argument/parameter" fluff entirely, which is consistent with at least half of the occurrences. |
Thanks for your help. |
A fine alternative if one doesn't want to build it locally, or can't, is to do it in one's own fork. |
This can be closed, as it's subsumed by the merged #6519. |
…shared], [util.smartptr.atomic.weak], [atomics.flag]: Reword Preconditions on memory_order values in a positive form