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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 100: Add a settings menu with a single preference for controlling sync frequency. #103

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jwir3
Copy link
Contributor

@jwir3 jwir3 commented Feb 22, 2014

I added a new 'Settings' menu to the Dashboard Activity which contains a single preference: the frequency, in minutes, of how often to synchronize repositories.

This works, but it's sort of a first pass at fixing this problem. Ideally, I think it would be nice to be able to sync with different options (e.g. every X minutes/hours, once per week, once a day, X day of the month, etc..). This is possible right now, but it requires that the user calculate the sync frequency to minutes, which is rather tedious for longer-term syncs.

@@ -25,3 +25,5 @@ out
.DS_Store
agit/artwork/rasters/
agit/gravatars

agit/lint.xml
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this needs to be part of this commit? Feel free to add a lint.xml in a separate PR if it's useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll remove this. I think it snuck in when I was importing it to Android Studio.

@rtyley
Copy link
Owner

rtyley commented Mar 2, 2014

Thanks for this submission - it's a good feature, but I think this dialog would be better as an exponential slider, or just a combo box with decent options (which I think is what you mean by this being a first pass):

screenshot_2014-03-02-12-29-49

I don't think users really need to be able to set their sync time to minute precision- a reasonable set of effective options would be:

  • every minute
  • every 5 minutes
  • every 15 minutes
  • every hour
  • every 4 hours
  • every day
  • every week

...and I can't imagine a situation where it would benefit the user to have longer sync periods.

Incidentally, the CLA for Agit is here, if you want to make a submission please make sure you specify which copyright option you choose:

https://github.com/rtyley/agit/wiki/Contribution-Agreement

@jwir3
Copy link
Contributor Author

jwir3 commented Mar 2, 2014

I've updated the pull request to remove the lint.xml from the .gitignore file. Additionally, I caught a problem with the AndroidManifest.xml file where I inadvertently changed the versionCode field, which I've now reverted.

I'll be happy to make the change outlined in your most recent comment, Roberto.

As for the question on whether I would assign the copyright to you or disclaim the copyright, and thus put the code in the public domain, is there really a difference? My suspicion is that by assigning the copyright to you, it would add the ability for you to go after someone for outright copyright infringement if the need arose, whereas the other option would not have this available. Is this correct? (There might be something else I'm missing, so I just wanted to make sure I understand the two options fully).

@rtyley
Copy link
Owner

rtyley commented Mar 2, 2014

I'll be happy to make the change outlined in your most recent comment, Roberto.

That's great ✨ For instance PocketCast does this, totally reasonably:

screenshot_2014-03-02-23-31-12 1

As for the question on whether I would assign the copyright to you or disclaim the copyright, and thus put the code in the public domain, is there really a difference?

Yeah, the reasoning is covered pretty fully here:

http://www.gnu.org/licenses/gpl-faq.html#AssignCopyright

@jwir3
Copy link
Contributor Author

jwir3 commented Mar 14, 2014

I think this is ready to be pulled. I've mimicked the example screen, and enabled a version of sync that happens at time-of-day using the AlarmManager.

Also, I hereby transfer copyright to Roberto Tyley for this feature to be included in agit.

@@ -43,6 +43,8 @@ the alternative [Git logo](http://henrik.nyh.se/2007/06/alternative-git-logo-and

* Bernd Hirschler - [German Translation](https://github.com/rtyley/agit/pull/35)

* Scott Johnson - [Sync Frequency UI](https://github.com/rtyler/agit/pull/103)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's @rtyley - not rtyler!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I'm very sorry. It was a typo... I'll change it as soon as I have a
chance.

~Scott
On Mar 15, 2014 3:56 PM, "Roberto Tyley" [email protected] wrote:

In CREDITS.markdown:

@@ -43,6 +43,8 @@ the alternative [Git logo](http://henrik.nyh.se/2007/06/alternative-git-logo-and

+* Scott Johnson - Sync Frequency UI

It's @rtyley https://github.com/rtyley - not rtyler!

Reply to this email directly or view it on GitHubhttps://github.com//pull/103/files#r10636607
.

jwir3 added a commit to jwir3/agit that referenced this pull request Mar 17, 2014
… PR rtyley#103.

The following items were changed:
  * Removed commented options to sync on a specific day of the week and sync on
    a specific day of the month.
  * Added additional possibilities to sync every: 5 minutes, 4 hours, daily (not
    at a specified time), and weekly.
… PR rtyley#103.

The following items were changed:
  * Removed commented options to sync on a specific day of the week and sync on
    a specific day of the month.
  * Added additional possibilities to sync every: 5 minutes, 4 hours, daily (not
    at a specified time), and weekly.
  * Added some summary text to the list preference in the SettingsActivity that
    displays the current sync setting, so the user doesn't have to click into
    the sub-screen to see which sync setting is selected.
@jwir3
Copy link
Contributor Author

jwir3 commented Mar 19, 2014

One thing I've noticed when testing this is that on repos that have a self signed cert, sync can't ask for permission. I'm not sure if this should be a separate ticket, but this hampers sync in these cases because there isn't a way to prevent agit from asking if the cert is OK. Perhaps there should be a cert storage mechanism, or something cooked into this ticket that allows users to just accept sync operations on a given repository once the first manual refresh has been done?

@jwir3
Copy link
Contributor Author

jwir3 commented Mar 31, 2014

@rtyley Aside from issue #107, this pull request is probably ready for review again. I have confirmed that on hosts that don't have the issue described in #107 (i.e. hosts that don't have a self-signed certificate), the sync options all work as expected.

@jwir3
Copy link
Contributor Author

jwir3 commented May 9, 2014

Review ping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants