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

Fix "iff" typo in Javadoc, comments and release notes #4502

Closed
wants to merge 4 commits into from

Conversation

Philzen
Copy link
Contributor

@Philzen Philzen commented Apr 26, 2024

While working with Jackson, i noticed this typo while exploring the API via JavaDoc.

If this gets merged, i can happily also assemble PRs for all other branches. Kindly advise if 2.13 and 2.14 will still see patch releases (latest Spring Boot 2.x branch still relies on 2.13).

@pjfanning
Copy link
Member

iff is a word - it means 'if and only if'.

https://en.wiktionary.org/wiki/iff#:~:text=(mathematics%2C%20logic)%20Short%20for%20if%20and%20only%20if.

So I see no issues in the existing docs. And even if we chose to use 'if', I can't see the issue being big enough to mean doing loads of patch releases. You must be aware that releases are not trivial.

@Philzen
Copy link
Contributor Author

Philzen commented Apr 26, 2024

iff is a word - it means 'if and only if'.

@pjfanning thanks for the clarification! After over 20 years of making a living by coding and a degree in CS, i'm always fascinated to be poked upon knowledge i apparently missed – may be due to the language barrier as a non-native english speaker.

Apologies if this felt like spamming – i merely wanted to contribute back to Jackson, which i believe is awesome, after recently having migrated a huge project over from GSON.

And even if we chose to use 'if', I can't see the issue being big enough to mean doing loads of patch releases. You must be aware that releases are not trivial.

I was not expecting such a small change to trigger any need for a new patch release – it could just transpire whenever a patch would be done. That being said, i'm happy to amend my commits (e.g. only fixing the public API javadoc and clariying using the term "if (and only if)") to whatever the team seems best or close my PR.

@JooHyukKim
Copy link
Member

What @pjfanning is saying is correct on that iff is valid word.
But the word unfortunately is easily mistaken as typo -- I've seen similar comments/PRs.

There is no issue with the docs, but I guess we may just push through replace with if and only if, to save time for the future?

@cowtowncoder
Copy link
Member

@JooHyukKim In this particular case, I am ok pushing back on PRs -- this has happened at least once before, but it is not super common source of confusion.
I am fine with usage of full "if (and only if)" for new comments, or when changing. But let's not change proactively here.

@cowtowncoder
Copy link
Member

Also: reading through couple of examples I think it is fair to say that plain "if" would work fine for many cases. I am not against changing wherever it makes sense. Just not as bulk replacement.

@cowtowncoder
Copy link
Member

@Philzen I think the patch reference was due to PR being against 2.15, and there not being other changes. We would not be releasing now patches from 2.15 just for Javadoc change(s), but would -- like you say -- only release them as part of patch whenever there'd be other functionality changes.
So it's all good.

Appreciate your submitting contributions for what seem like things to improve: "iff" has indeed been unknown to others before.

@Philzen
Copy link
Contributor Author

Philzen commented Apr 28, 2024

I am not against changing wherever it makes sense. Just not as bulk replacement.

So should i instead go ahead and submit separate PRs – or instead do a new one, thinking through each of the occurences wether iff actually applies? Happy to do either, so this is fixed once and for all and won't come up again in the future.

BTW – as soon as i understood that iff is actually a thing thanks to @pjfanning's comment i was actually contemplating to drop 0bcc0c8, as these comments are not part of the public API anyway.

@cowtowncoder
Copy link
Member

@Philzen it being OSS, it is up to you on what you want to work on, so although to me there isn't much value here (i.e. there are other fish to fry, big sea :) ), if you want to focus on improving readability, go for it!

And if so, yeah I'd prefer you go over cases and remove "iff" wherever "if" is as good a choice (or perhaps better -- I am not native speaker and my choices may well have been suboptimal).
I'll re-open this PR in case you want to use it as a base; if used, need to rebase to 2.18.

Or if you don't see any value, feel free to close.

@cowtowncoder cowtowncoder reopened this Apr 28, 2024
@Philzen Philzen changed the base branch from 2.15 to 2.18 May 17, 2024 20:07
@Philzen Philzen marked this pull request as draft May 17, 2024 20:45
@cowtowncoder
Copy link
Member

I think this can be closed, usage can be changed as part of other improvements as necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants