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

pullrequest correctly search linked targets based on pullrequestID #838

Merged
merged 3 commits into from
Sep 1, 2022

Conversation

olblak
Copy link
Member

@olblak olblak commented Aug 31, 2022

Signed-off-by: Olblak me@olblak.com

Fix #836
In release 0.30.0, Updatecli automatically detects which targets are related to a pullrequest spec, for generating the pullrequest body.

So

pullrequests:
  jenkins:
    title: Bump Jenkins Version to {{ source "jenkins" }}
    kind: github
    scmid: github2
    targets:
      - jenkins

becomes

pullrequests:
  jenkins:
    title: Bump Jenkins Version to {{ source "jenkins" }}
    kind: github
    scmid: github2

But in the code I passed the wrong id value the pullrequest scmID instead of the pullrequets ID
I didn't detect this issue because in my tests environment, it's usually the same value.

Test

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

cp <to_package_directory>
go test

Additional Information

Tradeoff

Potential improvement

@olblak olblak added the bug Something isn't working label Aug 31, 2022
@hervelemeur
Copy link
Contributor

hervelemeur commented Aug 31, 2022

I didn't detect this issue because in my tests environment, it's usually the same value.

Can tests be added to check this case?

@olblak
Copy link
Member Author

olblak commented Sep 1, 2022

I didn't detect this issue because in my tests environment, it's usually the same value.

Can tests be added to check this case?

Honestly I was wondering the same, because we need to try to open a PR.
I don't see an easy way other than adding a new e2e test

Signed-off-by: Olblak <me@olblak.com>
@olblak
Copy link
Member Author

olblak commented Sep 1, 2022

Here is one

@olblak olblak enabled auto-merge (squash) September 1, 2022 07:00
@olblak olblak disabled auto-merge September 1, 2022 07:00
@olblak olblak enabled auto-merge (squash) September 1, 2022 07:00
Signed-off-by: Olblak <me@olblak.com>
@olblak
Copy link
Member Author

olblak commented Sep 1, 2022

well e2e test allowed me to discover another bug :D

@olblak olblak disabled auto-merge September 1, 2022 10:46
@olblak olblak merged commit 5a4e367 into updatecli:main Sep 1, 2022
@olblak olblak added the resource-yaml Resource of kind YAML label Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working resource-yaml Resource of kind YAML
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pullRequests.go fails with panic: runtime error: index out of range [0] with length 0
2 participants