[DT-4] Auto-update feature #74

ycngadeu-coveo · 2019-06-12T22:23:02Z

Done:

Added an auto update feature
Added a no-update flag to skip the auto update

TODO:

Some tests
Refactoring
Better logging

coveralls · 2019-06-12T22:28:00Z

Pull Request Test Coverage Report for Build 951

3 of 97 (3.09%) changed or added relevant lines in 2 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage decreased (-4.0004%) to 39.757%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
cli.go	3	18	16.67%
update_unix.go	0	79	0.0%

Files with Coverage Reduction	New Missed Lines	%
cli.go	1	73.87%

Totals
Change from base Build 923:	-4.0004%
Covered Lines:	392
Relevant Lines:	986

💛 - Coveralls

julienduchesne

Great start! I have a few comments for you

cli.go

tgf_auto_update.go

cli.go

tgf_auto_update.go

cli.go

auto_update.go

go.mod

update_unix.go

julienduchesne

This looks good to me!
How about you @jocgir ?

julienduchesne · 2019-06-18T22:52:11Z

Wait up. Let's review Linux and Windows independently, please. The other PR also should've been merged independently.

Essentially, you shouldn't bunch up 3 PRs worth of stuff in a single one. I will merge the update script PR into master but you need to split up linux and Windows stuff

julienduchesne · 2019-06-19T11:05:57Z

Here I merged the linux script update. To be clear, open:
a PR for the Windows update script
a PR for the linux auto update
a PR where you add Windows auto update to the linux codebase

I think there will be enough discussion to warrant three PRs

update_unix.go

cli.go

julienduchesne

Sad trombone
I'm getting
environment: line 33: /home/jduchesne/repos/tgf/tgf: Text file busy

We may have to do the Windows trick even on linux

update_unix.go

This reverts commit 7b88ddb.

This reverts commit ac783c9.

This reverts commit d3984ba.

cli.go

update_unix.go

julienduchesne · 2019-06-25T20:54:50Z

cli.go

@@ -208,5 +217,21 @@ func (app *TGFApplication) Run() int {
 		Printf("tgf v%s\n", version)
 		return 0
 	}
+
+	if app.DeleteCopy {
+		os.Remove(getHomeFileName("tgf"))


give it a hidden name. not tgf

julienduchesne · 2019-06-25T20:55:22Z

cli.go

+	}
+
+	if app.CopyPath != "" {
+		cloneExecutableAt(app.CopyPath)


This should delete the copy if it exists

julienduchesne · 2019-06-25T20:56:34Z

update_unix.go

+func getCurrentExecutablePath() string {
+	executablePath, err := os.Executable()
+	if err != nil {
+		printWarning("Error getting executable path", err)


If there's an error. This will return "" and then the next command will fail

julienduchesne · 2019-06-25T20:57:47Z

update_unix.go

+	"path/filepath"
+)
+
+// RunUpdate runs the update on the current tgf executable


This deserves to be way better documented. An outsider has to be able to figure out what will happen with this doc

julienduchesne · 2019-06-25T20:58:05Z

update_unix.go

+	cmd.Run()
+}
+
+func cloneExecutableAt(newExecutablePath string) string {


This is used in another file. It shouldn't be private and it should be documented

julienduchesne · 2019-06-25T20:59:38Z

cli.go

+	var dueForUpdate = lastRefresh(autoUpdateFileName) > 2*time.Hour
+	if app.DoUpdate && dueForUpdate && RunUpdate() {
+		touchImageRefresh(autoUpdateFileName)
+		ReRunCopy()


I'd call that ForwardCallToNewVersion. It's long but descriptive

julienduchesne · 2019-06-25T21:04:26Z

touch.go

@@ -10,8 +10,12 @@ import (
 )

 func getTouchFilename(image string) string {
+	return getHomeFileName(util.EncodeBase64Sha1(image))


There's still a whole bunch of duplicated logic and no clear distinction where the code should go (this file or update_*.go). I suggest putting all homedir handling to a file called homedir.go. You could then have simple commands that only need the filename as parameter:
Read
Write (This can replace touch)

[DT-4] - [Done] - Pre refactoring

dce14fc

ycngadeu-coveo requested review from jocgir, julienduchesne, dblanchette and jcbedard June 12, 2019 22:23

julienduchesne suggested changes Jun 13, 2019

View reviewed changes

cli.go Outdated Show resolved Hide resolved

tgf_auto_update.go Outdated Show resolved Hide resolved

cli.go Outdated Show resolved Hide resolved

cli.go Outdated Show resolved Hide resolved

tgf_auto_update.go Outdated Show resolved Hide resolved

cli.go Outdated Show resolved Hide resolved

ycngadeu-coveo requested a review from AbdulRahmanAlHamali June 13, 2019 12:33

Yann Cedric Ngadeu added 5 commits June 13, 2019 10:50

[DT-4] - Make no update flag positive

02faf5c

[DT-4] - Specified unix implementation

a6966c5

[DT-4] - Using app.Degug for logging

322113d

[DT-4] - Implemented tgf update cooldown period

a31d4c8

[DT-4] - Specified unix and windows implementation

a654d9c

ycngadeu-coveo requested a review from julienduchesne June 13, 2019 16:57

ycngadeu-coveo added the feature label Jun 13, 2019

julienduchesne suggested changes Jun 13, 2019

View reviewed changes

auto_update.go Outdated Show resolved Hide resolved

go.mod Show resolved Hide resolved

update_unix.go Outdated Show resolved Hide resolved

[DT-4] - Refactoring

2246942

julienduchesne previously approved these changes Jun 17, 2019

View reviewed changes

ycngadeu-coveo dismissed julienduchesne’s stale review via daa82f2 June 18, 2019 19:09

ycngadeu-coveo removed request for dblanchette, AbdulRahmanAlHamali, jocgir and jcbedard June 18, 2019 19:19

julienduchesne force-pushed the feature/dt-4-auto-update branch from 3d2b514 to 1cbcb91 Compare June 19, 2019 11:03

julienduchesne force-pushed the feature/dt-4-auto-update branch from 1cc76ac to 2246942 Compare June 19, 2019 16:50

Merge branch 'master' into feature/dt-4-auto-update

8f2c1b3

ycngadeu-coveo requested a review from julienduchesne June 19, 2019 20:36

julienduchesne suggested changes Jun 20, 2019

View reviewed changes

update_unix.go Outdated Show resolved Hide resolved

update_unix.go Outdated Show resolved Hide resolved

julienduchesne reviewed Jun 20, 2019

View reviewed changes

cli.go Outdated Show resolved Hide resolved

[DT-4] - More refactoring - Move line back

f13b6cb

ycngadeu-coveo requested review from julienduchesne, jocgir, AbdulRahmanAlHamali, dblanchette and jcbedard June 20, 2019 15:48

julienduchesne suggested changes Jun 20, 2019

View reviewed changes

update_unix.go Outdated Show resolved Hide resolved

[DT-4] - Using Combined output to capture std err

d4d0ab8

ycngadeu-coveo removed request for jocgir, AbdulRahmanAlHamali, dblanchette and jcbedard June 20, 2019 18:31

Yann Cedric Ngadeu added 7 commits June 20, 2019 14:31

[DT - 4] - Changing unix tgf update strategy

d3984ba

[DT - 4] - Rename variables

ac783c9

[DT-4] - Add delete step at the end of the call

7b88ddb

Revert "[DT-4] - Add delete step at the end of the call"

0d8a05b

This reverts commit 7b88ddb.

Revert "[DT - 4] - Rename variables"

c32d487

This reverts commit ac783c9.

Revert "[DT - 4] - Changing unix tgf update strategy"

5ca8437

This reverts commit d3984ba.

[DT-4] - New update strategy

597d050

julienduchesne suggested changes Jun 25, 2019

View reviewed changes

cli.go Outdated Show resolved Hide resolved

cli.go Outdated Show resolved Hide resolved

julienduchesne suggested changes Jun 25, 2019

View reviewed changes

cli.go Outdated Show resolved Hide resolved

cli.go Outdated Show resolved Hide resolved

update_unix.go Outdated Show resolved Hide resolved

update_unix.go Outdated Show resolved Hide resolved

update_unix.go Outdated Show resolved Hide resolved

update_unix.go Outdated Show resolved Hide resolved

Yann Cedric Ngadeu added 2 commits June 25, 2019 16:07

[DT-4] - Resolve comments

538a227

[DT-4] - Delete tgf copy in home directory

5ea2fcd

ycngadeu-coveo requested review from julienduchesne, jocgir and AbdulRahmanAlHamali June 25, 2019 20:49

julienduchesne suggested changes Jun 25, 2019

View reviewed changes

ycngadeu-coveo closed this Jun 26, 2019

ycngadeu-coveo deleted the feature/dt-4-auto-update branch June 26, 2019 18:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DT-4] Auto-update feature #74

[DT-4] Auto-update feature #74

ycngadeu-coveo commented Jun 12, 2019

coveralls commented Jun 12, 2019 •

edited

Loading

julienduchesne left a comment

julienduchesne left a comment

julienduchesne commented Jun 18, 2019 •

edited

Loading

julienduchesne commented Jun 19, 2019

julienduchesne left a comment

julienduchesne Jun 25, 2019

julienduchesne Jun 25, 2019

julienduchesne Jun 25, 2019

julienduchesne Jun 25, 2019

julienduchesne Jun 25, 2019

julienduchesne Jun 25, 2019

julienduchesne Jun 25, 2019

[DT-4] Auto-update feature #74

[DT-4] Auto-update feature #74

Conversation

ycngadeu-coveo commented Jun 12, 2019

coveralls commented Jun 12, 2019 • edited Loading

Pull Request Test Coverage Report for Build 951

💛 - Coveralls

julienduchesne left a comment

Choose a reason for hiding this comment

julienduchesne left a comment

Choose a reason for hiding this comment

julienduchesne commented Jun 18, 2019 • edited Loading

julienduchesne commented Jun 19, 2019

julienduchesne left a comment

Choose a reason for hiding this comment

julienduchesne Jun 25, 2019

Choose a reason for hiding this comment

julienduchesne Jun 25, 2019

Choose a reason for hiding this comment

julienduchesne Jun 25, 2019

Choose a reason for hiding this comment

julienduchesne Jun 25, 2019

Choose a reason for hiding this comment

julienduchesne Jun 25, 2019

Choose a reason for hiding this comment

julienduchesne Jun 25, 2019

Choose a reason for hiding this comment

julienduchesne Jun 25, 2019

Choose a reason for hiding this comment

coveralls commented Jun 12, 2019 •

edited

Loading

julienduchesne commented Jun 18, 2019 •

edited

Loading