-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[bitbucket] enable projects #7251
Conversation
76375dc
to
709a825
Compare
709a825
to
7f7fa96
Compare
/werft run with-clean-slate-deployment 👍 started the job as gitpod-build-at-bb-projects.5 |
Just found an issue with the cloneURL.
... on it. |
7f7fa96
to
f45d583
Compare
@jldec, just fixed the issue with the wrong |
Looking at this now! 👀 |
f45d583
to
4d7bc93
Compare
/lgtm |
LGTM label has been added. Git tree hash: 514b037a80e1280716e3a1b70cc1e6a0ba261051
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jldec Associated issue: #5980 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although a bit late but submitting some pending review comments since these could be helpful.
Thanks @AlexTugarev! Feels great to now support Bitbucket repositories as projects. 🔮
commit: { | ||
sha: branch.target?.hash!, | ||
author: branch.target?.author?.user?.display_name!, | ||
authorAvatarUrl: branch.target?.author?.user?.links?.avatar?.href, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: So jealous the commit author avatar is displayed for Bitbucket repositories but not for GitLab and GitHub repositories. 😭
question: Definitely out of the scope of this PR, but is this as simple to fix for GitLab and GitHub repositories? Happy to also open a follow up issue for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately, nope! they don't carry it like here.
}); | ||
} | ||
|
||
return branches; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: Although branches are successfully retrieved, we
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi: Opened #7366 to track this. Cc @AlexTugarev @jldec
const response = await api.user.listPermissionsForRepos({ | ||
q: `repository.full_name="${owner}/${repoName}"` | ||
}) | ||
return !!response.data?.values && response.data.values[0]?.permission === "admin"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: This could be considered out of the scope of this PR, but in contrast with GitLab and GitHub repositories, adding a Bitbucket repository
🍊 🍊 🍊 🍊
thought: This is also interesting to test and make it clearler if separating the steps adding a project and enabling prebuilds makes sense. Re-posting from #7031 (comment) for visibility:
Eventually we will most probably need to separate project addition and project configuration steps so that these can be done separastely. 💭
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I'll have a look into this. Thanks @gtsiolis!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi: Opened #7367 to track this. Cc @AlexTugarev @jldec
Resolves #5980
This also adds a test of permissions on webhook installations.