Refactor Go Merlin #1

a268wang · 2021-12-15T18:58:08-05:00

a268wang commented

2021-12-15 18:58:08 -05:00

Changes made

wrote some tests
split the common package into config, logger, and sync
moved the unix socket things from merlin.go into its own package
made some adjustments (splitting methods, moving stuff) to make it easier to test

Note: at the moment the drone.io is failing because the of some time zone stuff

Todo

write unit tests for the sync package
test the program as a whole

#### Changes made - wrote some tests - split the common package into config, logger, and sync - moved the unix socket things from merlin.go into its own package - made some adjustments (splitting methods, moving stuff) to make it easier to test Note: at the moment the drone.io is failing because the of some time zone stuff #### Todo - write unit tests for the sync package - test the program as a whole

a268wang added 8 commits 2021-12-15 18:58:09 -05:00

8d572a0c3f split into packages

69fbcfb13d arthur and config testing and fixes

3f541ec31f add drone.io

bc9f5f1dbd test drone.io

de0e175c92 test drone.io

continuous-integration/drone Build is passing Details

5049b40ad9 attempt to trigger drone.io

continuous-integration/drone Build is passing Details

ea292c82f1 trigger drone.io

continuous-integration/drone/pr Build is failing Details

continuous-integration/drone Build is failing Details

4a76506b35 update .drone.yml to actually run the tests

d278liu requested changes 2021-12-16 23:10:39 -05:00

d278liu left a comment

lgtm except the things in sync.go

merlin/sync/command.go

													
				@ -0,0 +246,4 @@

						return

					}

					if repo.PasswordFile != "" {

d278liu commented

2021-12-16 23:07:00 -05:00

same here, some sync types already append the password file - and how the password file should be added to args depends on the type of sync, so probably would remove this too

a268wang commented

2021-12-16 23:27:01 -05:00

I agree with this, I'll remove it shortly

a268wang marked this conversation as resolved

merlin/sync/command.go

													
				@ -0,0 +251,4 @@

						args = append(args, "--password-file", filename)

					}

					args = append(args, buildRsyncHost(repo), localDir)

d278liu commented

2021-12-16 22:52:51 -05:00

don't think you want to be appending rsync host and localDir here too. each sync type already appends it.

also different sync types have different ways of building the host (some needs to be the daemon style host and some needs to be a ssh host)

don't think you want to be appending rsync host and localDir here too. each sync type already appends it. also different sync types have different ways of building the host (some needs to be the daemon style host and some needs to be a ssh host)

a268wang commented

2021-12-16 23:19:20 -05:00

Huh all this stuff was actually moved over from the startSync function

https://git.csclub.uwaterloo.ca/public/mirror/src/branch/go/merlin/common/sync.go#L256-L270

I'm thinking of keeping a modified version of part to make sure that startSync will immediately exit if merlin cannot mkdir the buildDownloadDir

Huh all this stuff was actually moved over from the startSync function https://git.csclub.uwaterloo.ca/public/mirror/src/branch/go/merlin/common/sync.go#L256-L270 I'm thinking of keeping a modified version of part to make sure that startSync will immediately exit if merlin cannot mkdir the buildDownloadDir

d278liu commented

2021-12-16 23:47:34 -05:00

yeah my bad about the original sync.go i think i forgot to remove them

a268wang marked this conversation as resolved