reposense / RepoSense

Contribution analysis tool for Git repositories
https://reposense.org
MIT License
250 stars 153 forks source link

[#1832] Move report generation timestamp #2241

Closed jedkohjk closed 3 months ago

jedkohjk commented 3 months ago

Resolves #1832

Proposed commit message

Shift report generation timestamp up

Removes footer so that there is no inconsistency regarding 
the presence/absence of a footer between panels.

Rephrases message so it does not look like it is part of title.

Other information

Before:

before-top

before-bottom

After:

after-top

after-bottom

damithc commented 3 months ago

Thanks for the PR @jedkohjk hmm... I don't think the timestamp/version etc. fits in the top/middle of the report either, as it is just a 'small print' that doesn't deserve space in high-value screen real estate.

jedkohjk commented 3 months ago

Thanks for the review @damithc

Hmm, I agree that the version and user guide would fit a duplicated footer, as suggested in the original issue. But I felt like the timestamp may not feel like it fits on the right panel as the timestamp is for the entire report. I'm not sure if that makes sense.

As of now, I don't really have any other suggestions. Perhaps we could shift the reposense version and user guide back to the footer and duplicate it, and leave the time stamp on top?

Something like this (only for the left panel, the bottom of the right panel should have the reposense link in what I'm proposing, but I made this with microsoft paint):

Screenshot

Without the blue hyperlinks, the timestamp is less attention grabbing.

ckcherry23 commented 3 months ago

CS2103/T students may prefer finding the timestamp more easily on the top/middle as it can help them figure out why certain commits have not been updated yet.

However, a timestamp at the top may not be ideal for our other use cases, such as creating a project code portfolio.

jedkohjk commented 3 months ago

Hi, I made a separate pull request, #2243 as another alternative. There, I left the timestamp as a footer on the left panel, and duplicated only the reposense links.

jedkohjk commented 3 months ago

Hi, should I close this PR as CP3108A is coming to an end for me?

github-actions[bot] commented 3 months ago

The following links are for previewing this pull request: