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

Fix cluster pull request statuses #1064

Merged
merged 7 commits into from Nov 19, 2021
Merged

Fix cluster pull request statuses #1064

merged 7 commits into from Nov 19, 2021

Conversation

sarataha
Copy link
Contributor

@sarataha sarataha commented Nov 10, 2021

Closes: weaveworks/weave-gitops-enterprise#250

What changed?

Updated Cluster struct to get the correct PR type

Why?

To differentiate between different PR types whether it's creation or deletion

How did you test it?

Tested locally + unit tests

Release notes

Documentation Changes

@sarataha sarataha added the bug Something isn't working label Nov 10, 2021
@sarataha sarataha requested a review from foot November 10, 2021 22:44
Copy link
Contributor

@foot foot left a comment

Choose a reason for hiding this comment

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

Works great!

It would be really neat show the URL there too. Maybe another column called STATUS_MESSAGE for now? (sounds a little bit awkward). We could do this in a follow up PR too

c.Status = "Deletion PR"
}

fmt.Fprintf(w, "%s\t%s", c.Name, c.Status)
fmt.Fprintf(w, "%s\t%s\t%s", c.Name, c.Status, c.PullRequest.Url)
Copy link
Contributor

Choose a reason for hiding this comment

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

🙌 , maybe only if c.status is creation/deletion PR. e.g. I guess leave it blank for "clusterFound" etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! 👌 Thank you Simon 🙏

@sarataha sarataha merged commit c36e813 into main Nov 19, 2021
@sarataha sarataha deleted the fix-pr-status branch November 19, 2021 09:18
joshri pushed a commit that referenced this pull request Nov 19, 2021
* Fix change PR status based on PR type

* Fix lint

* Return pr url

* return pr url for creation and deletion only

* Update tests

* Fix tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CLI] gitops CLI doesn't differentiate between capi cluster pull request statuses
2 participants