PR listing: add --fields & expose additional fields #415

Merged
6543 merged 9 commits from justusbunsi/tea:feature/show-labels-for-pulls into master 1 year ago
Collaborator

This PR adds the --fields flag to tea pr ls (#342), and exposes more fields specific to the PullRequest type:

   --fields value, -f value   Comma-separated list of fields to print.
                              Available values:
                              index,state,author,author-id,url,title,body,mergeable,base,base-commit,head,diff,patch,created,updated,deadline,assignees,milestone,labels,comments
                              (default: "index,title,state,author,milestone,updated,labels")

This PR adds the `--fields` flag to `tea pr ls` (#342), and exposes more fields specific to the `PullRequest` type: ``` --fields value, -f value Comma-separated list of fields to print. Available values: index,state,author,author-id,url,title,body,mergeable,base,base-commit,head,diff,patch,created,updated,deadline,assignees,milestone,labels,comments (default: "index,title,state,author,milestone,updated,labels") ```
justusbunsi added 1 commit 1 year ago
continuous-integration/drone/pr Build is passing Details
4132aabf32
Add labels to pulls list
noerw commented 1 year ago
Collaborator

Thanks!
#400 also proposes to implements this, though in a different way.
As described in that PR, there's problems with that approach, and currently I tend to remove the tea pr ls changes from #400, instead aiming to add more filters to the API in Gitea & SDK in the long run, in the end favoring this PR.

Thanks! #400 also proposes to implements this, though in a different way. As described in that PR, there's problems with that approach, and currently I tend to remove the `tea pr ls` changes from #400, instead aiming to add more filters to the API in Gitea & SDK in the long run, in the end favoring this PR.
noerw added the kind/enhancement label 1 year ago
noerw added this to the v0.9.0 milestone 1 year ago
justusbunsi added 1 commit 1 year ago
continuous-integration/drone/pr Build is passing Details
0e18e1130b
Support additional flags for output
Poster
Collaborator

As per Discord chat with @noerw: I added support for additional fields to make it more similar to the issues command. Are there any other fields that would be nice to have for PRs?

Also, I'm not totally happy with these changes. The formatting related code for issues and pulls are ~100% duplicates.

As per Discord chat with @noerw: I added support for additional fields to make it more similar to the `issues` command. Are there any other fields that would be nice to have for PRs? Also, I'm not totally happy with these changes. The formatting related code for issues and pulls are ~100% duplicates.
justusbunsi changed title from Show labels for pulls to Support additional fields for pulls 1 year ago
justusbunsi added 1 commit 1 year ago
continuous-integration/drone/pr Build is passing Details
77b1244531
Switch icons for bool formatting
noerw reviewed 1 year ago
for _, label := range x.Labels {
if _, ok := labelMap[label.ID]; !ok {
labelMap[label.ID] = formatLabel(label, !isMachineReadable(output), "")
labelMap[label.ID] = label
noerw commented 1 year ago
Collaborator

the idea here was to do the work of formatLabel() for each label only once. now labelMap does not really serve a purpose.

but I guess this map was premature optimization anyway, as the total label count for a call of printIssues is unlikely to exceed 1000 individual labels anyway, in most cases more like 20-50.
so I'd remove labelMap and this loop

the idea here was to do the work of `formatLabel()` for each label only once. now `labelMap` does not really serve a purpose. but I guess this map was premature optimization anyway, as the total label count for a call of `printIssues` is unlikely to exceed 1000 individual labels anyway, in most cases more like 20-50. so I'd remove labelMap and this loop
Poster
Collaborator

Wouldn't it be counter productive to remove such performance improvement, even if it's currently a possible premature one? I thought it would be good to have all the format<...>() function calls encapsulated inside FormatField. In fact, I didn't realize the performance optimization taking place here even though there is a comment mentioning it. 😬

If you don't mind, I'd rather keep this label map for both issues and pulls.

Wouldn't it be counter productive to remove such performance improvement, even if it's currently a possible `premature` one? I thought it would be good to have all the `format<...>()` function calls encapsulated inside `FormatField`. In fact, I didn't realize the performance optimization taking place here even though there is a comment mentioning it. 😬 If you don't mind, I'd rather keep this label map for both issues and pulls.
noerw marked this conversation as resolved
justusbunsi added 1 commit 1 year ago
continuous-integration/drone/pr Build is passing Details
1349925308
Remove useless kind field for pulls
justusbunsi added 1 commit 1 year ago
continuous-integration/drone/pr Build is passing Details
c3e7885412
Reimplement performance optimization for labels
Poster
Collaborator

We could partially integrate #258 in this PR. conflicts? is already represented by the additional field mergeable. base/head branch are available and only needs to be added to the optional fields.
Regarding reviews (count) we'd need to do an extra rest request to retrieve all reviews of that PR to count them.

We could partially integrate #258 in this PR. `conflicts?` is already represented by the additional field `mergeable`. `base/head branch` are available and only needs to be added to the optional fields. Regarding `reviews` (count) we'd need to do an extra rest request to retrieve all reviews of that PR to count them.
justusbunsi changed title from Support additional fields for pulls to WIP: Support additional fields for pulls 1 year ago
justusbunsi changed title from WIP: Support additional fields for pulls to Support additional fields for pulls 1 year ago
noerw added 4 commits 1 year ago
noerw approved these changes 1 year ago
noerw changed title from Support additional fields for pulls to PR listing: add --fields & expose additional fields 1 year ago
techknowlogick approved these changes 1 year ago
6543 approved these changes 1 year ago
6543 merged commit 3cf084cb96 into master 1 year ago
justusbunsi deleted branch feature/show-labels-for-pulls 1 year ago

Reviewers

noerw approved these changes 1 year ago
techknowlogick approved these changes 1 year ago
6543 approved these changes 1 year ago
continuous-integration/drone/pr Build is passing
The pull request has been merged as 3cf084cb96.
Sign in to join this conversation.
Loading…
There is no content yet.