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

feat: add branch endpoint supporting GET and DELETE #635

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

bethesque
Copy link
Member

@bethesque bethesque commented Sep 11, 2023

Adds a branch resource /pacticipants/{pacticipant}/branches/{branch} that supports GET and DELETE.

Branches and versions exist independently of each other, and deleting one does not cause the other to be deleted. The branch_version resource is the association of a branch with a version (branches and versions have a many to many relationship via branch_versions).

Deleting a branch_version resource does not delete the version, but just removes the version from the branch.

Deleting a pacticipant version deletes all the associated branch_versions, but does not cause any branches to be deleted.

To be consistent with existing behaviour, a DELETE to the branch resource should delete the branch, and all the associated branch versions, but leave the pacticipant versions.

This is not very useful though. The main usecase for deleting a branch is to do a clean up after a merge, and remove all the associated pacticipant versions and their pacts/verifications. Perhaps we could support a query parameter or header that indicated whether or not to do delete all the versions or not.

Perhaps I'm thinking too fine grained, and there should be a post merge callback endpoint that does multiple things.

Would love some thoughts on this.

@mefellows
Copy link
Member

Adds a branch resource /pacticipants/{pacticipant}/branches/{branch} that supports GET and DELETE.
Branches and versions exist independently of each other, and deleting one does not cause the other to be deleted. The branch_version resource is the association of a branch with a version (branches and versions have a many to many relationship via branch_versions).
Deleting a branch_version resource does not delete the version, but just removes the version from the branch.
Deleting a pacticipant version deletes all the associated branch_versions, but does not cause any branches to be deleted.
To be consistent with existing behaviour, a DELETE to the branch resource should delete the branch, and all the associated branch versions, but leave the pacticipant versions.

I think this makes sense and keeping it consistent with other domain resources seems reasonable.

If I understand this correctly, would it actually solve the use case? Truthfully, the underlying need is really about usability - the feedback is usually about removing out-of-date data from / decluttering the UI so the user can focus on what's important. If the version/results/etc. still existed but were orphaned and not showing up on the dashboard (and various places in PactFlow), I don't think that would really be an issue.

This may not be the best behaviour to rely on in the future (e.g. the UI may change to show different sets of data),

This is not very useful though. The main usecase for deleting a branch is to do a clean up after a merge, and remove all the associated pacticipant versions and their pacts/verifications. Perhaps we could support a query parameter or header that indicated whether or not to do delete all the versions or not.
Perhaps I'm thinking too fine grained, and there should be a post merge callback endpoint that does multiple things.

A callback API that does this would indeed be nicer. If I properly understand the above, there will be some versions left over (that don't have contracts associated with it) and verification results left over.

In case it helps with naming etc., we could perhaps start to consider an "SCM" related set of APIs that we could use to support other related use cases (e.g. a commit status, PR open, build failure, label).

@bethesque bethesque merged commit 1bb6088 into master Sep 12, 2023
19 checks passed
@bethesque bethesque deleted the feat/delete-branch branch September 12, 2023 21:18
@bethesque bethesque linked an issue Sep 13, 2023 that may be closed by this pull request
@mefellows
Copy link
Member

Relates to #628

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for deleting branches
2 participants