Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Activity to view the Git commit log #1056

Merged
merged 16 commits into from Aug 28, 2020

Conversation

@Nosweh
Copy link
Contributor

@Nosweh Nosweh commented Aug 26, 2020

馃摙 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates

馃摐 Description

The GitLogActivity is accessible via the action bar menu ("View commit log") if the password repo is a Git repository. The Activity displays basic information about the commits in the repository using a RecyclerView, after internally performing a git-log command. This information is

  1. the short commit hash (first 8 characters)
  2. the commit's short message
  3. the commit date and time.

(Also see the screenshots.)

I'm not quite sure if we should also display the author of the commits because I suppose for most users they will be always the same. Or is it a common use case to have multiple authors contribute to the same pass repo? It would not be a problem to add it, though. The (newly added) intermediate data class GitCommit does contain the author's name already; I just decided to remove it from the UI for now, for simplicity. What do you all think?

馃挕 Motivation and Context

See open issue #284.

馃挌 How did you test it?

To test the UI and usage of a 'casual' user, I opened a password Git repo in APS with a couple of commits and then opened the new Activity to view the "Commit log". I performed this test on three devices:

  • an AVD phone with 1080x1920 resolution at 420 ppi running Android 10,
  • an AVD tablet with 2560x1800 resolution at xhdpi running Android 10, and
  • a physical phone with 1080x2280 resolution at 432 ppi running Android 9.

To test the performance of acquiring the commit metadata, I opened a password Git repo in APS with 1,000 commits and then viewed the commit log. I performed this test on a physical, mid-range 2018 phone running Android 9 (+ on an AVD, to be precise). I noticed no noticeably longer delay in the user experience compared to performing other user actions in the app.

馃摑 Checklist

  • I formatted the code with the IDE's reformat action (Ctrl + Shift + L/Cmd + Shift + L)
  • I reviewed submitted code
  • I added a CHANGELOG entry if applicable

馃敭 Next steps

We could make the items in this new Git commit log interactive to show more detailed information about the commit (author, changed files/passwords etc.)

馃摳 Screenshots / GIFs

"Commit log" (GitLogActivity) on a phone:

Action bar menu with "Show commit log" (on Git repo):

"Commit log" (GitLogActivity) on a tablet:

Suggestion (not included in the original state of this PR) for a row layout, in case the commit author's name should be included, too (possibly with a smaller message font):

Nosweh and others added 5 commits Aug 20, 2020
Previously, some of the inputs in the PasswordCreationActivity would be
off-screen and inaccessible, when the screen height was reduced by
rotating the device from portrait to landscape orientation or opening
the on-screen keyboard, for example. The visible portion of the inputs
could not be changed.

To fix this, make the PasswordCreationActivity's layout scrollable. This
way, inputs that are off-screen can be brought into view by scrolling.

Especially the 'Extra content' input is way more accessible now, as it
would also be off-screen previously, even when it was being focused
for editing. Now, it will be automatically brought into view again after
the on-screen keyboard opens.
* develop:
  build: bump version
  Update CHANGELOG
  Revert "Reland symlink support (#1020)"

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
The GitLogActivity is accessible via the action bar menu ("View commit
log") if the password repo is a git repository. The activity displays
basic information about the commits in the repository using a
RecyclerView, after internally performing a git-log command. This
information is 1. the short commit hash, 2. the commit's short message,
3. the commit date and time. Other information is omitted from the UI
as of this commit.
@Nosweh Nosweh requested review from fmeum, msfjarvis and Skrilltrax as code owners Aug 26, 2020
Copy link
Member

@fmeum fmeum left a comment

Thank you very much for the contribution, this looks pretty good and useful to me!

I only left a few comments that are mostly nits.

CHANGELOG.md Outdated Show resolved Hide resolved
app/src/main/res/menu/main_menu_git.xml Outdated Show resolved Hide resolved
Copy link
Member

@msfjarvis msfjarvis left a comment

This is looking extremely good! I just have a couple nits here and there.

msfjarvis added 2 commits Aug 26, 2020
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Copy link
Member

@msfjarvis msfjarvis commented Aug 26, 2020

I had some time to spare while checking this out so I've addressed the simpler stuff, leaving you more time to focus on the more involved aspects. Let me know if you have questions about any of the changes I made.

@msfjarvis msfjarvis added the feature label Aug 26, 2020
@msfjarvis msfjarvis added this to the 1.12.0 milestone Aug 26, 2020
@msfjarvis msfjarvis linked an issue that may be closed by this pull request Aug 26, 2020
@msfjarvis msfjarvis self-assigned this Aug 26, 2020
msfjarvis and others added 6 commits Aug 26, 2020
The commit log is accessible via a button in thte
GitConfigActivity.

The GitConfigActivity now gives info about the state of HEAD as in
git-status.

Additionally, rename the "Hackish tools" and "Git utils" labels to fit
the funtionality better.
@Nosweh
Copy link
Contributor Author

@Nosweh Nosweh commented Aug 28, 2020

I have now made the commit log accessible via the GitConfigActivity, changed the HEAD info there to use git-status wording and changed the "Hackish tools" label to "Utilities" as well as the "Git utils" label to "Local Git config & utilities". I am sure the last label is not ideal, but then again, I could not think of a shorter label that would give enough information about the GitConfigActivity for someone who is searching for specific functionality and is unfamiliar with the app.

I tested the HEAD info once with HEAD pointing to master and once with HEAD pointing to a specific commit directly.

I also added the two extensions to RevCommit you asked for, @FabianHenneke .



Copy link
Member

@fmeum fmeum left a comment

Looks great, works great, thanks a lot!

I only left two suggested changes that will help us when we finally get to refactor PasswordRepository.

@fmeum fmeum requested a review from msfjarvis Aug 28, 2020
@fmeum
fmeum approved these changes Aug 28, 2020
@Nosweh
Copy link
Contributor Author

@Nosweh Nosweh commented Aug 28, 2020

Alright, should be all done now.

@msfjarvis msfjarvis merged commit 0f0d199 into android-password-store:develop Aug 28, 2020
5 checks passed
5 checks passed
test-pr (23, freeDebug)
Details
test-pr (23, nonFreeDebug)
Details
test-pr (29, freeDebug)
Details
test-pr (29, nonFreeDebug)
Details
WIP Ready for review
Details
@msfjarvis
Copy link
Member

@msfjarvis msfjarvis commented Aug 28, 2020

Congratulations! 馃帄

@Nosweh Nosweh deleted the Nosweh:feature/git-log branch Aug 28, 2020
@Nosweh
Copy link
Contributor Author

@Nosweh Nosweh commented Aug 28, 2020

Thanks, I'm glad I could contribute. And thanks for your support!

msfjarvis added a commit that referenced this pull request Aug 29, 2020
* develop:
  Add Activity to view the Git commit log (#1056)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can鈥檛 perform that action at this time.