Skip to content
This repository has been archived by the owner on Nov 27, 2024. It is now read-only.

rest: add initial support for PATCH #315

Merged
merged 10 commits into from
Oct 14, 2024
Merged

rest: add initial support for PATCH #315

merged 10 commits into from
Oct 14, 2024

Conversation

filariow
Copy link
Member

Signed-off-by: Francesco Ilario filario@redhat.com

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Oct 11, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@filariow filariow force-pushed the rest-patch-initial-support branch from ea81941 to d5d6536 Compare October 11, 2024 15:10
@filariow
rest: add initial support for PATCH
6c0ed76 
Signed-off-by: Francesco Ilario <filario@redhat.com>
@filariow filariow force-pushed the rest-patch-initial-support branch 2 times, most recently from 6f3f6af to 809eafb Compare October 11, 2024 15:41
@filariow
enable patch endpoint
8499492 
Signed-off-by: Francesco Ilario <filario@redhat.com>
@filariow filariow force-pushed the rest-patch-initial-support branch from 809eafb to 8499492 Compare October 11, 2024 15:53
@filariow
add patch endpoint e2e test
5c4f878 
Signed-off-by: Francesco Ilario <filario@redhat.com>
@filariow filariow marked this pull request as ready for review October 11, 2024 16:05
@filariow
remove @wip
38e1e8b 
Signed-off-by: Francesco Ilario <filario@redhat.com>
@filariow
fix user patch step
c5f41bd 
Signed-off-by: Francesco Ilario <filario@redhat.com>
sadlerap
sadlerap reviewed Oct 11, 2024
View reviewed changes
server/core/workspace/workspace_patch.go Outdated
Comment on lines 83 to 85
if command.PatchType != types.MergePatchType {
return nil, fmt.Errorf("unsupported patch type: %s", command.PatchType)
}
Copy link
Member

Choose a reason for hiding this comment

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

maybe add a test so that other patch types are rejected for now?

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea, let me add them

Copy link
Member Author

Choose a reason for hiding this comment

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

done in bd8a3d3

@filariow
Copy link
Member Author

/hold adding strategic-merge

@filariow
add strategic merge support
d001c6a 
Signed-off-by: Francesco Ilario <filario@redhat.com>
@filariow
Copy link
Member Author

/unhold strategic-merge added in d001c6a

@filariow filariow requested a review from sadlerap October 14, 2024 13:26
sadlerap
sadlerap approved these changes Oct 14, 2024
View reviewed changes
Copy link
Member

@sadlerap sadlerap left a comment

Choose a reason for hiding this comment

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

I think this is okay. I have some concerns about the soundness of the semantics, but I think this should be fine for a demo.

@openshift-ci openshift-ci bot added the lgtm label Oct 14, 2024
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Oct 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: filariow, sadlerap

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@filariow
Copy link
Member Author

/unhold

@openshift-merge-bot openshift-merge-bot bot merged commit a91eb55 into konflux-workspaces:main Oct 14, 2024
17 checks passed
@filariow filariow deleted the rest-patch-initial-support branch October 14, 2024 21:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sadlerap sadlerap sadlerap approved these changes

@dperaza4dustbit dperaza4dustbit Awaiting requested review from dperaza4dustbit

Assignees

@sadlerap sadlerap

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@filariow @sadlerap