Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
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.

Sign up for GitHub

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

Jump to bottom

feat: gitness/harness prebuild webhook #1199

Merged
merged 15 commits into from
Oct 21, 2024
Merged

feat: gitness/harness prebuild webhook #1199

merged 15 commits into from
Oct 21, 2024

Conversation

rutikt7066
Copy link
Contributor

@rutikt7066 rutikt7066 commented Oct 4, 2024 •
edited
Loading

gitness/harness prebuild webhook

Description

This PR add Prebuild hook for gitness/harness.
/claim #997

ee.mp4

@algora-pbc algora-pbc bot mentioned this pull request Oct 6, 2024
Closed
@rutikt7066 rutikt7066 force-pushed the gitness-prebuild-webhook branch 4 times, most recently from 35c4f09 to 979f99a Compare October 7, 2024 20:30
@rutikt7066 rutikt7066 marked this pull request as ready for review October 7, 2024 20:30
@rutikt7066 rutikt7066 requested a review from a team as a code owner October 7, 2024 20:30
@rutikt7066 rutikt7066 marked this pull request as draft October 8, 2024 05:21
@rutikt7066 rutikt7066 marked this pull request as ready for review October 8, 2024 08:06
@rutikt7066 rutikt7066 changed the title feat: gitness/harness prebuild webhook (WIP) feat: gitness/harness prebuild webhook Oct 8, 2024
Tpuljak
Tpuljak requested changes Oct 8, 2024
View reviewed changes
pkg/gitprovider/gitness.go Outdated Show resolved Hide resolved
pkg/gitprovider/gitnessclient/gitness_client.go Outdated Show resolved Hide resolved
@rutikt7066 rutikt7066 requested a review from Tpuljak October 8, 2024 10:27
@rutikt7066 rutikt7066 force-pushed the gitness-prebuild-webhook branch from 18060a3 to c5c72a3 Compare October 8, 2024 11:07
@rutikt7066 rutikt7066 requested a review from Tpuljak October 8, 2024 11:56
Tpuljak
Tpuljak requested changes Oct 8, 2024
View reviewed changes
Copy link
Member

@Tpuljak Tpuljak left a comment

Choose a reason for hiding this comment

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

I'll move the PR to draft until you address the comment below. Please provide a video where you test the PR with commit interval and trigger files.

pkg/gitprovider/gitness.go Show resolved Hide resolved
@Tpuljak Tpuljak marked this pull request as draft October 8, 2024 12:21
@rutikt7066 rutikt7066 force-pushed the gitness-prebuild-webhook branch 2 times, most recently from b1f7c4d to 33b369a Compare October 8, 2024 19:51
@rutikt7066
Copy link
Contributor Author

done.mp4

Tpuljak
Tpuljak requested changes Oct 9, 2024
View reviewed changes
pkg/gitprovider/gitness.go Show resolved Hide resolved
@rutikt7066
Copy link
Contributor Author

@Tpuljak Gitness api for listing all commits has param called after we can pass hash to that param and it will return the commit after that hash. So we don't need to list all commit and filter it.

@Tpuljak
Copy link
Member

Tpuljak commented Oct 9, 2024

@Tpuljak Gitness api for listing all commits has param called after we can pass hash to that param and it will return the commit after that hash. So we don't need to list all commit and filter it.

But how are you supposed to know how many commits are made between initialSha and currentSha if you don't even use currentSha in the function 😄

Forget the prebuild trigger for now, just focus on GetCommitsRange and make it return the number of commits between initialSha and currentSha

@rutikt7066
Copy link
Contributor Author

@Tpuljak Gitness api for listing all commits has param called after we can pass hash to that param and it will return the commit after that hash. So we don't need to list all commit and filter it.

But how are you supposed to know how many commits are made between initialSha and currentSha if you don't even use currentSha in the function 😄

Forget the prebuild trigger for now, just focus on GetCommitsRange and make it return the number of commits between initialSha and currentSha

@Tpuljak I think currentSha means the latest one. Right?

@Tpuljak
Copy link
Member

Tpuljak commented Oct 9, 2024

@Tpuljak I think currentSha means the latest one. Right?

It means exactly that sha. It might be the latest one but it might not be. It's not important if it's the last one of not.

@rutikt7066
Copy link
Contributor Author

@Tpuljak oh I thought it will be latest one every time. My bad.

@rutikt7066
Copy link
Contributor Author

rutikt7066 commented Oct 9, 2024 •
edited
Loading

@Tpuljak Updated the changes. Please Take a look I will upload the video soon

@rutikt7066
Copy link
Contributor Author

@Tpuljak

clideo_editor_5970bf36aec245e193096b90f9d59372.mp4

@rutikt7066 rutikt7066 requested a review from Tpuljak October 10, 2024 09:39
@rutikt7066
Copy link
Contributor Author

@idagelic Sure I will check whats happening.

@rutikt7066
git provder list bug fix
18c9603 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
Copy link
Contributor Author

@idagelic Solved this issue. We were not setting it while listing.

@rutikt7066
Copy link
Contributor Author

@idagelic Apologies for the delayed commit. I accidentally pushed the commit to my fork's main when I meant to tag you. I just noticed it and have now committed the changes to this PR.

@rutikt7066 rutikt7066 requested a review from Tpuljak October 16, 2024 11:34
@rutikt7066
Copy link
Contributor Author

Hi @idagelic, I just wanted to kindly check in on the PR status and see if there's any feedback when you have a moment. Thanks in advance! 🙏

idagelic
idagelic requested changes Oct 17, 2024
View reviewed changes
Copy link
Member

@idagelic idagelic left a comment

Choose a reason for hiding this comment

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

This doesn't work for url's containing capital letters because you didn't clean up the repository url. Attach another video when fixed

pkg/gitprovider/gitness.go Outdated Show resolved Hide resolved
@rutikt7066
Merge branch 'main' of https://github.com/the-johnwick/daytona
1b9dc5c
@rutikt7066
init
4356136 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
webhook
4806d15 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
PasreGitEventData
0076102 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
bug-fix
427e33b 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
createwebhook map to type webhook
5e09e9d 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
request changes: argumet name change
e0b22c2 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
bug-fix
9edc171 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
bug-fix
d85c8a6 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
fix getCommitRange
d172965 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
getCommitRange
72cd525 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
requested-change: math.abs() insted if else comp
9426c9a 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066
url clean up
2ccb7b4 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066 rutikt7066 force-pushed the gitness-prebuild-webhook branch from 6c65449 to 2ccb7b4 Compare October 17, 2024 12:57
@rutikt7066
bug fix: clone url
aa0ac16 
Signed-off-by: the-johnwick <jiosupprt270@gmail.com>
@rutikt7066 rutikt7066 requested a review from idagelic October 18, 2024 19:55
idagelic
idagelic approved these changes Oct 21, 2024
View reviewed changes
Copy link
Member

@idagelic idagelic left a comment

Choose a reason for hiding this comment

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

Nice work

@rutikt7066
Copy link
Contributor Author

Nice work

Thank You so much 😊

@Tpuljak Tpuljak merged commit 5ce468c into daytonaio:main Oct 21, 2024
12 checks passed
@rutikt7066 rutikt7066 deleted the gitness-prebuild-webhook branch October 21, 2024 10:56
@Rutik7066 Rutik7066 mentioned this pull request Feb 5, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@idagelic idagelic idagelic approved these changes

@Tpuljak Tpuljak Tpuljak approved these changes

Assignees

@rutikt7066 rutikt7066

Labels
🙋 Bounty claim
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rutikt7066 @Tpuljak @idagelic