PR Builds tab added to project dashboard #5807
Conversation
saadmk11
added
the
PR: work in progress
saadmk11
changed the title
saadmk11
removed
the
PR: work in progress
|
Isn't "Pull Request" a term fixed to GitHub itself? I think that GitLab uses "Merge Request" for example (https://docs.gitlab.com/ee/user/project/merge_requests/). Can we come up with a general and descriptive name for this? |
|
@humitos Github and Bitbucket uses |
|
I wonder if we should use |
| @@ -45,6 +45,8 @@ <h1> | |||
|
|
|||
| <li class="{{ builds_active }}"><a href="{{ project.get_builds_url }}">{% trans "Builds" %}</a></li> | |||
|
|
|||
| <li class="{{ pr_builds_active }}"><a href="{{ project.get_pr_builds_url }}">{% trans "PR Builds" %}</a></li> | |||
There was a problem hiding this comment.
This should only show up if the user has any PR/external builds.
There was a problem hiding this comment.
@ericholscher Updated the PR
saadmk11
force-pushed
the
gsoc-19-pr-builder
branch
2 times, most recently
from
05951cf to
5a05b6c
Compare
|
This can now be updated against current |
|
Long term, why can't this all be under the |
Should this be done now or after the |
|
I guess why do we need subsections or a separate page at all? Could the normal and PR builds be intermingled in a single view (eventually)? Having the two joined seems like it will be better UX for the user. |
|
If we do that then we don't need this PR at all. We can just update the |
|
Well, i have been outside any planning for this feature, but from the outside, it seems like a lot of duplicated UI and code. It's not immediately apparent why having a separate section for PR builds is better than combining the two build types under a single listing. If we don't have a good answer to why these build types need to be separated in our UI, then I think combining them is the best answer. We can apply state and filtering based on the build type if we need to. Is there a strong reason to split these up? Why are we moving away from the original implementation that used a single listing view? |
|
The original thinking was because external builds will come from other users, and will be much more likely to fail. We don't want it to confuse users who see them in their normal build listings. I do think we could get away with a filter on the page, but having a separate top-level listing seemed to make the most sense for now. I was also taking inspiration from Travis, which splits them out:
I don't feel too strongly though, this just seemed easiest and least confusing for users to start. |
|
Gotcha. It's a little better when named "Pull Requests", as that communicates the page's purpose fairly well, but I'm still not sure separating is the best UX for us however. For comparison, CircleCI and GitLab have singular list of builds. If we consider the perspective of the maintainer:
If our main concern is polluting the build list, we can add filtering later (probably defaulting to filtering PR builds out on the build list). By merging the two lists, we worry about optimizing later, and we can avoid complicating the implementation now, so we don't have to rip out redundant code and templates later. If they truly need to be separate, "External builds" should be renamed. We have the repo type, so we can match "Pull requests" vs "Merge requests" etc on the tab based on what provider the project uses. The function of the page would at least be clearer to maintainers this way. I'd still be -0 on this path though, it seems like we're making more UI for us to maintain without a clear benefit. |
|
I think we've decided not to do this for now. We can come back to this PR if we need to in the future, but I'm closing it for now. |
This is a continuation of #5779
This will add a tab for PR Builds in the project dashboard.