search

tf / redmine_merge_request_links

Display links to associated Gitlab merge requests and GitHub pull requests on Redmine's issue page.
https://www.redmine.org/plugins/redmine_merge_request_links
MIT License
35 stars 26 forks source link

Fix layout in Redmine 4.1, add timestamps to merge requests #28

Open alexandermalfait opened 4 years ago

alexandermalfait commented 4 years ago

Relating to https://github.com/tf/redmine_merge_request_links/issues/26

alexandermalfait commented 4 years ago

Example layout:

image

tf commented 3 years ago

Sorry for not getting back earlier. This might be a good compromise. I find the new design a bit dense and harder to read. Have you considered keeping the items multi line or turning the list into a table to align columns? I also asked for design feedback internally since this is a plugin we use quite a lot.

CI is failing since the migration is not compatible with all supported Redmine versions. You can check the other migrations for the correct syntax.

How does the timestamp look like for old merge requests that were added before the timestamp columns were added?

Also if we take this route, we will no longer depend on the custom hook that currently needs to be added via a patch. The corresponding files could then be removed and the readme would need to be updated accordingly.

aviav commented 3 years ago

Kudos for coming up with a fix! I also use this quite a lot, but currently, we are on an older version of Redmine, so that's why the issue hasn't yet occurred for us.

For me, this change would introduce the problem that it is easy to lose track which line I am in while scanning the long line, and then mis-identify which PR/MR ID is open, which is merged, or which is closed. As your example shows, MR titles could be quite similar and are sometimes even identical which makes it even more likely to slip between lines. In this use case, having ID and MR status close together makes it much easier to quickly and reliably tell what is the status of which. In practice, I use these IDs to distinguish between MRs when communicating with others, and often, only a minority of MRs/PRs on a ticket is actually open, so having these bits of information close together is great for my use case.

A potential fix for this problem might be to alternate background color slightly e.g. between odd and even lines -- but still, the lines would be pretty long, full of information, and for me, the information that, in the best case, would be close together is now far apart.

In principle, I like the layout that we had up until now, because it didn't increase the need to scroll by too much. The change introduced in this PR also achieves this well. By the multi-column approach with comments etc. in one column and MRs/PRs in another column, however, lines in both columns became shorter and easier to scan.

Still, for the tab layout, which I think is a very nice addition and which I look forward to using, I don't even know whether it's possible to maintain all those advantages at the same time. Maybe just having MR/PR information on all the tabs would be fine for now, though we should fix the styles in this case. I'd expect that it could turn out that at least I only need PR/MR info on the tab where the discussion takes place, if I want to avoid having to switch tabs -- but this is only a guess, since I haven't used these tabs yet. How do you use this information, usually?