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.

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

Restore mistakenly removed iffs #2387

Merged
merged 1 commit into from
Sep 12, 2019
Merged

Restore mistakenly removed iffs #2387

merged 1 commit into from
Sep 12, 2019

Conversation

kuzkry
Copy link
Contributor

@kuzkry kuzkry commented Aug 12, 2019

This partially reverts #2356 (it's not 100% revert, I made some modifications to prevent too long lines).

TL;DR: Wary as usual @adambadura (thanks mate, all credit to you ;) ) found out that I made a mistake in #2356 by thinking iff was a typo of "if", whereas it really stands for "if and only if".

For more, please follow the discussion started in #2356.

@adambadura
Copy link
Contributor

@kuzkry, I think it would be better to use if and only if instead of iff. This will prevent the same misunderstanding in future.

However, if comment lines are broken at some column (which seems to be a somewhat flexible rule) changing iff to if and only if cannot be done by simple find-and-replace and requires a manual check of each case to ensure that line length limits are observed.

@kuzkry kuzkry changed the title Revert mistakenly removed iffs WIP: Revert mistakenly removed iffs Aug 12, 2019
@kuzkry
Copy link
Contributor Author

kuzkry commented Aug 12, 2019

@kuzkry, I think it would be better to use if and only if instead of iff. This will prevent the same misunderstanding in future.

However, if comment lines are broken at some column (which seems to be a somewhat flexible rule) changing iff to if and only if cannot be done by simple find-and-replace and requires a manual check of each case to ensure that line length limits are observed.

Ok, done. I'm keeping this WIP, as I would like you to make sure it's like you wanted it to be. @adambadura, please take some time and be very precise when pointing me to lines that you still consider too long.

Commits and their logic:
1a6d736 is find and replace
2f39f43 is what clang formatting tool thinks about it
f7b850e is a bunch of my propositions

When we're done, I will simply fix the last two and withdraw WIP from the PR title.

@adambadura
Copy link
Contributor

Seems OK to me. I have no particular needs about the line breaking. I just noticed that in general line breaking is used. Although the length limit didn't seem consistent.

@kuzkry
Copy link
Contributor Author

kuzkry commented Aug 12, 2019

Thank you @adambadura!

@kuzkry
Copy link
Contributor Author

kuzkry commented Aug 12, 2019

Temporarily closing this to avoid a merge conflict with another pull request.

@kuzkry kuzkry closed this Aug 12, 2019
@kuzkry kuzkry changed the title WIP: Revert mistakenly removed iffs Revert mistakenly removed iffs Aug 16, 2019
@kuzkry kuzkry reopened this Aug 16, 2019
@kuzkry kuzkry changed the title Revert mistakenly removed iffs Restore mistakenly removed iffs Aug 16, 2019
Due to confusion arisen from "iff" standing for "if and only if",
this commit uses the latter.
shaindelschwartz added a commit that referenced this pull request Sep 12, 2019
PiperOrigin-RevId: 268693457
shaindelschwartz added a commit that referenced this pull request Sep 12, 2019
PiperOrigin-RevId: 268693457
@shaindelschwartz shaindelschwartz merged commit 7bd4a7f into google:master Sep 12, 2019
@kuzkry kuzkry deleted the iff branch September 12, 2019 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants