Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[variant.get] Consistently place comma after "otherwise" #2221

Merged
merged 1 commit into from Jun 29, 2018
Merged

[variant.get] Consistently place comma after "otherwise" #2221

merged 1 commit into from Jun 29, 2018

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented Jun 28, 2018 •
edited

This is done for local consistency. There are 3 other occurrences within the subclause that use comma.

@CaseyCarter
Copy link
Contributor

Now that P0788 has been applied, It may be better to turn these \requires elements into \mandates and drop the "Otherwise the program is ill-formed." altogether.

@JohelEGP
Copy link
Contributor Author

Then there wouldn't be consistency within the sibling subclauses. Have the editors come to an agreement on when to use of the new elements when doing drive-by fixes?

@CaseyCarter
Copy link
Contributor

Then there wouldn't be consistency within the sibling subclauses.

That's a good enough argument to convince me.

Have the editors come to an agreement on when to use of the new elements when doing drive-by fixes?

Not to my knowledge.

jensmaurer
jensmaurer approved these changes Jun 28, 2018
View reviewed changes
Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the existing text to the new concepts/contract forms of expressing requirements and semantics should be done as a separate paper (possibly closely coordinated with the editors) and not as a drive-by for a simple added comma.

@jwakely
Copy link
Member

jwakely commented Jun 28, 2018

should be done as a separate paper

That paper is on my TODO list.

@zygoloid zygoloid merged commit 492e6a7 into cplusplus:master Jun 29, 2018
@JohelEGP JohelEGP deleted the variant_get branch June 29, 2018 23:01
@JohelEGP
Copy link
Contributor Author

@jwakely Will your paper's proposed changes be a 1:1 transformation, or will it include "fixes"? For example, https://wg21.link/time.duration.io#1 should have been a Remarks: element as I suggest in https://wg21.link/LWG3125. The new calendar and timezone library has many of those, so I'm wondering if I should submit a DR on those and any other I find while browsing the standard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jensmaurer jensmaurer jensmaurer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@JohelEGP @CaseyCarter @jwakely @jensmaurer @zygoloid