Fix: repo-config for pullRequests.draft
should not be wiped when combined with _unset_ config
#3719
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a bug that came in with the initial introduction of the
pullRequests.draft
config setting:The bug was that when combining global & local repo config, the result of combining
pullRequests.draft = true
with a config with no setting forpullRequests.draft
would be thatpullRequests.draft
was unset (ie given the valueNone
, rather thanSome(true)
.Here's an example of where this was a problem in the wild:
Cause
This arose because of the behaviour of
.mapN()
, which I hadn't really thought through:scala-steward/modules/core/src/main/scala/org/scalasteward/core/repoconfig/PullRequestsConfig.scala
Line 66 in 373d5a7
...in retrospect, it's not too surprising that
.mapN()
on twoOption
s only produces a populatedSome
if both the inputs areSome
- which is not the behaviour we want here. For us, if only one of the configs has a value set, we definitely want to use that value.Fix
In the end, the neatest way I could find to get the desired behaviour was to remove the thing stopping us from having a
Monoid[Option[Boolean]]
- ie that there is no generalMonoid[Boolean]
(there is no one correct way to combine twoBoolean
s). We can create a specific one for this case, that saysBoolean
s should be combined with logical-OR
(rather thanAND
, for instance) - and only expose it in the very narrow scope of where we want to use it.