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

Improve StreamMapToOptionalGet Refaster template documentation #203

Merged
merged 2 commits into from
Aug 19, 2022

Conversation

rickie
Copy link
Member

@rickie rickie commented Aug 18, 2022

No description provided.

@rickie rickie added this to the 0.1.1 milestone Aug 18, 2022
@rickie rickie requested a review from Stephan202 August 18, 2022 12:21
@Stephan202 Stephan202 force-pushed the rossendrijver/javadoc_improvement branch from 451643c to 493db87 Compare August 19, 2022 06:52
Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Rebased and added a tiny tweak.

@Stephan202
Copy link
Member

Suggested commit message:

Improve `StreamMapToOptionalGet` Refaster template documentation (#203)

Copy link
Contributor

@nathankooij nathankooij left a comment

Choose a reason for hiding this comment

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

Just an open question but we can merge the PR already. :)

* Within a stream's map operation unconditional {@link Optional#orElseThrow()} calls can be
* avoided.
*
* <p><strong>Warning:</strong> this rewrite rule is not completely behavior preserving. The
Copy link
Contributor

Choose a reason for hiding this comment

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

In most cases since we're in a stream already I suppose flattening the Optional would be the user's intention, but I wonder how we could also convey/educate users with the changes that are made for them. Now, the process is such that if they don't like a change, they will ping @rickie to ask, but it might be nice to also think of an automated solution for templates similar to how there's documentation for bug patterns.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, have been pondering the same. Ideally the Refaster check can emit custom error messages, just like other BugCheckers. We might be able to use annotations for this; TBD.

Next to that we should generate Github pages (or equivalent) and link to that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also bounced a few ideas offline just now with @rickie. Annotations would be super nice -- we could also use it to indicate whether some things are behavior preserving or not, and I'm sure we can think of many more nice categories. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

There are a lot of cool ideas and options. Would be really nice to get stuff like that done 😍.

@rickie rickie changed the title Update StreamMapToOptionalGet Javadoc Improve StreamMapToOptionalGet Refaster template documentation Aug 19, 2022
@rickie rickie merged commit a58630b into master Aug 19, 2022
@rickie rickie deleted the rossendrijver/javadoc_improvement branch August 19, 2022 11:33
@rickie rickie added the documentation A documentation update label Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation A documentation update
Development

Successfully merging this pull request may close these issues.

3 participants