Pull #18631: Add OptionalRulesRecipes #18629 #18627 #18625 #18622#18631
Pull #18631: Add OptionalRulesRecipes #18629 #18627 #18625 #18622#18631Pankraz76 wants to merge 1 commit into
OptionalRulesRecipes #18629 #18627 #18625 #18622#18631Conversation
OptionalRulesRecipes #18629 #18627 #18625 #18622OptionalRulesRecipes #18629 #18627 #18625 #18622
a7892c9 to
88f5288
Compare
88f5288 to
96523bd
Compare
romani
left a comment
There was a problem hiding this comment.
Comment out this rule to make it clear it was considered, but disabled due to personal preferences
|
|
||
| if (firstSentence.isPresent()) { | ||
| if (containsForbiddenFragment(firstSentence.get())) { | ||
| if (containsForbiddenFragment(firstSentence.orElseThrow())) { |
There was a problem hiding this comment.
I am not mentally ready for this update.
Let's disable this rule for now. New generation of maintainers might think differently, they can activate it.
Not now
There was a problem hiding this comment.
i understand but just dont give so much attention.
looking into the numbers this argument makes not much sense, as we already have this all around 48 times:
so its already there what you dont like, meaning you already have the mental capacity for this. also its about adopting to new standards and evolve.
please reconsider.
If this would be the only once i would agree but currently the argument makes no sense, sorry.
96523bd to
82b23b5
Compare
82b23b5 to
373a5fd
Compare
|
please merge and apply convention fix the gap. we have real issues but not such cosmetic stuff. just obey, its nothing new.... |
|
plz reply to merge or i make comment that you dont like 4 new items when there are already 48 items around. its kind of nonsense wasting again. lets try to stop starting and start finighing @romani |
| - tech.picnic.errorprone.refasterrules.MapRulesRecipes | ||
| - tech.picnic.errorprone.refasterrules.MicrometerRulesRecipes | ||
| - tech.picnic.errorprone.refasterrules.MockitoRulesRecipes | ||
| - tech.picnic.errorprone.refasterrules.OptionalRulesRecipes |
There was a problem hiding this comment.
disable it with comment "current maintainers do not like orElseThrow"
There was a problem hiding this comment.
then have to revert all other.
with me only consequent changes going to happen.
might having to escalate this with other contributors to help out.
| getPropertyVersionFromItsJavadoc(propertyJavadoc); | ||
|
|
||
| if (specifiedPropertyVersionInPropertyJavadoc.isPresent()) { | ||
| sinceVersion = specifiedPropertyVersionInPropertyJavadoc.get(); |
There was a problem hiding this comment.
this actually a good trivial change having same behaviour but different code being finally aligned.
373a5fd to
c36a630
Compare
Pankraz76
left a comment
There was a problem hiding this comment.
kind of escalating, asking for feedback.
| .filter(JavadocUtil::isJavadocComment) | ||
| .findFirst(); | ||
| return javadoc.isPresent() | ||
| && MATCH_INHERIT_DOC.matcher(javadoc.orElseThrow()).find(); |
There was a problem hiding this comment.
this is kind of a joke to me.
| - tech.picnic.errorprone.refasterrules.MapRulesRecipes | ||
| - tech.picnic.errorprone.refasterrules.MicrometerRulesRecipes | ||
| - tech.picnic.errorprone.refasterrules.MockitoRulesRecipes | ||
| # current maintainer (romani) do not understand java properly; don't "like" orElseThrow. |
There was a problem hiding this comment.
There was a problem hiding this comment.
i would like to have a consistent way not some crazy random way. having 48 times orElseThrow but dont allow any further without any (valid) argument given?
c36a630 to
b9e5673
Compare
|
sorry for going so extra, thanks. actually this the first commit i see from you. |
Pull #18631: Add
OptionalRulesRecipes#18629 #18627 #18625 #18622