Remove "state": "OPEN" to fix updating of titles

[timja]

Fixes #866

Test

To test this pull request, you can run the following commands:

gh repo clone timja/updatecli-test
cd updatecli-test
updatecli apply

echo 'abcd=1' > my-file.properties
updatecli apply

Additional Information

Tradeoff

None that I can think of this seems like it was unnecessary, other code handles creating a new pull request if no open ones exist

#866 (comment)

Potential improvement

I've raised a ticket with GitHub to get clarification

[lemeurherve]

Thanks!

[olblak]

Thanks for the PR

I don't think a closed pull request would get opened any more is that okay?
Otherwise we could run the mutation twice, once with state and once with title and body.

In the past, we found it annoying that if someone accidentally close a PR, it doesn't get reopened by Updatecli. This led us to miss some update as the branch would kept being updated but we wouldn't notice it. So I would prefer doing the mutation twice

[timja]

Testing out the current code, it seems to create a new pull request rather than updating the existing branch, it's not finding it. Trying to see why but seems my current worry wasn't an issue?

(e.g. timja/updatecli-test#7)

[timja]

This isn't finding closed PRs:

updatecli/pkg/plugins/scms/github/pullrequest.go

Line 437 in 3898f8e

func (p *PullRequest) getRemotePullRequest() error {

So it opens a new PR (with the same branch), and all works how you would expect

[timja]

because of this (hardcoded search for state open):

updatecli/pkg/plugins/scms/github/pullrequest.go

Line 469 in 3898f8e

} `graphql:"pullRequests(baseRefName: $baseRefName, headRefName: $headRefName, last: 1, states: [OPEN])"`

Ok all seems fine and understood, cc @olblak

[olblak]

Ok all seems fine and understood, cc @olblak

Is this PR ready to be merged?
I'll trust your testing, as I don't have the time atm

[olblak]

I had a look to the previous PR I did on this topic to refresh my head and your PR seems great.
If you are, I am ready to merge this one

[timja]

Is this PR ready to be merged?

Yes I've tested many different methods and it's ready.

GitHub support is raising the issue to the engineering team.