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

Safe Upload #923

Merged
merged 6 commits into from
Jun 25, 2019
Merged

Safe Upload #923

merged 6 commits into from
Jun 25, 2019

Conversation

auxves
Copy link
Contributor

@auxves auxves commented Jun 25, 2019

Short description of what this resolves:

This PR enables "safe" upload functionality.

Changes proposed in this pull request:

  • Check if contents of gist are identical or newer to local settings and don't upload if they are (unless forceUpload is turned on)
  • Implement forceUpload

Fixes: #350 #316

How Has This Been Tested?

Tested by uploading to gist with contents older or identical to the gist.

Checklist:

  • I have read the contribution guidelines.
  • My change requires a change to the documentation and GitHub Wiki.
  • I have updated the documentation and Wiki accordingly.

@shanalikhan shanalikhan changed the base branch from v3.3.1 to v3.4.0 June 25, 2019 10:27
Copy link
Owner

@shanalikhan shanalikhan left a comment

Choose a reason for hiding this comment

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

This is v3.4.0 worthy. Base branch updated.

src/service/github.service.ts Outdated Show resolved Hide resolved
src/sync.ts Show resolved Hide resolved
package.nls.*.json
----
Reorder keys to be alphabetical

sync.ts
----
Notify that the upload was cancelled

github.service.ts
----
Update function name to IsGistNewer
@shanalikhan shanalikhan merged commit e1552d0 into shanalikhan:v3.4.0 Jun 25, 2019
@auxves auxves deleted the safe-upload branch June 25, 2019 11:55
@shanalikhan
Copy link
Owner

Tested by uploading to gist with contents older or identical to the gist.

Can you try with other scenario by pushing from one computer and then pushing from second computer ( without pulling ) , It should not upload as first computer has pushed the most updated settings.

@auxves
Copy link
Contributor Author

auxves commented Jun 25, 2019

@shanalikhan I have just tested that and it works.

@shanalikhan
Copy link
Owner

image

Gist LINK : https://gist.github.com/shanalikhan/e86911443d3f81be6a66699603aee6dc

It should not ask to force upload because lastUpload is Null and in cloudSettings settings date is old.

@auxves
Copy link
Contributor Author

auxves commented Jul 2, 2019

@shanalikhan This will be fixed in #937.

@shanalikhan
Copy link
Owner

Can you fix it in v3.4.0 branch? We can ship it in this week.

@shanalikhan
Copy link
Owner

shanalikhan commented Jul 3, 2019

Ubuntu, Standard Latest Code version. Build package and installed that package and test results

For #316
I'm uploading the same settings again and again and it is uploading.
It should not upload the settings on second upload and prompt that settings are the same and ask user to enable forceUpload to perform upload.

Second, autoUpload doesnt seems to working at my end. Can you test autoUpload scenarios at your end by testing on package and install locally.

@auxves auxves mentioned this pull request Jul 3, 2019
1 task
@shanalikhan
Copy link
Owner

Can you test autoUpload scenarios at your end by testing on package and install locally.

Any update on this ?

So the PR you have send will not allow user to upload the settings second time if there is not any difference between first upload content ?

@auxves
Copy link
Contributor Author

auxves commented Jul 5, 2019

If there is no difference between remote and local settings, a message box will be shown to the user informing them about it and giving them the option to enable forceUpload. If they enable it, the upload will continue.

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

Successfully merging this pull request may close these issues.

2 participants